From 9c725803a67f6d39db09fadf75ba707bf381a34d Mon Sep 17 00:00:00 2001 From: rahim Date: Sat, 31 Jan 2026 19:03:43 +1100 Subject: [PATCH] refactor(store): simplify create store implementations (#361) --- packages/store/src/lit/create-store.ts | 18 ++++---- packages/store/src/lit/index.ts | 4 +- .../store/src/lit/mixins/combined-mixin.ts | 10 ++--- .../{attach-mixin.ts => container-mixin.ts} | 6 +-- packages/store/src/lit/mixins/index.ts | 4 +- .../store/src/lit/mixins/provider-mixin.ts | 6 +-- ...-mixin.test.ts => container-mixin.test.ts} | 12 +++--- .../lit/mixins/tests/provider-mixin.test.ts | 26 +++++------ .../store/src/lit/mixins/tests/types.test.ts | 12 +++--- .../store/src/lit/tests/create-store.test.ts | 22 +++++----- packages/store/src/react/context.tsx | 9 ---- packages/store/src/react/create-store.tsx | 43 ++++--------------- 12 files changed, 69 insertions(+), 103 deletions(-) rename packages/store/src/lit/mixins/{attach-mixin.ts => container-mixin.ts} (93%) rename packages/store/src/lit/mixins/tests/{attach-mixin.test.ts => container-mixin.test.ts} (65%) diff --git a/packages/store/src/lit/create-store.ts b/packages/store/src/lit/create-store.ts index c3c348ec..6039f6f6 100644 --- a/packages/store/src/lit/create-store.ts +++ b/packages/store/src/lit/create-store.ts @@ -7,7 +7,7 @@ import type { AnyFeature, UnionFeatureRequests, UnionFeatureState, UnionFeatureT import type { StoreConfig, StoreConsumer, StoreProvider } from '../core/store'; import { Store } from '../core/store'; -import { createStoreAttachMixin, createStoreMixin, createStoreProviderMixin } from './mixins'; +import { createContainerMixin, createProviderMixin, createStoreMixin } from './mixins'; export const contextKey = Symbol('@videojs/store'); @@ -37,10 +37,10 @@ export interface CreateStoreResult { * * @example * ```ts - * class MyProvider extends StoreProviderMixin(LitElement) {} + * class MyProvider extends ProviderMixin(LitElement) {} * ``` */ - StoreProviderMixin: >(Base: T) => T & Constructor>; + ProviderMixin: >(Base: T) => T & Constructor>; /** * Mixin that auto-attaches slotted media elements (requires store from context). @@ -49,10 +49,10 @@ export interface CreateStoreResult { * * @example * ```ts - * class MyControls extends StoreAttachMixin(LitElement) {} + * class MyControls extends ContainerMixin(LitElement) {} * ``` */ - StoreAttachMixin: >(Base: T) => T & Constructor>; + ContainerMixin: >(Base: T) => T & Constructor>; /** * Context for consuming store in controllers. @@ -156,8 +156,8 @@ export function createStore( return new Store(config); } - const StoreProviderMixin = createStoreProviderMixin(context, create); - const StoreAttachMixin = createStoreAttachMixin(context); + const ProviderMixin = createProviderMixin(context, create); + const ContainerMixin = createContainerMixin(context); const StoreMixin = createStoreMixin(context, create); class StoreController { @@ -208,8 +208,8 @@ export function createStore( return { StoreMixin, - StoreProviderMixin, - StoreAttachMixin, + ProviderMixin, + ContainerMixin, context, create, StoreController, diff --git a/packages/store/src/lit/index.ts b/packages/store/src/lit/index.ts index 296e5f7f..464aadd4 100644 --- a/packages/store/src/lit/index.ts +++ b/packages/store/src/lit/index.ts @@ -15,9 +15,9 @@ export { createStore } from './create-store'; // Mixin factories (for advanced use cases) export { - createStoreAttachMixin, + createContainerMixin, + createProviderMixin, createStoreMixin, - createStoreProviderMixin, } from './mixins'; export type { StoreSource } from './store-accessor'; // StoreAccessor (for custom controllers) diff --git a/packages/store/src/lit/mixins/combined-mixin.ts b/packages/store/src/lit/mixins/combined-mixin.ts index fe10bbad..b05e3324 100644 --- a/packages/store/src/lit/mixins/combined-mixin.ts +++ b/packages/store/src/lit/mixins/combined-mixin.ts @@ -5,8 +5,8 @@ import type { AnyFeature, UnionFeatureTarget } from '../../core/feature'; import type { Store, StoreProvider } from '../../core/store'; -import { createStoreAttachMixin } from './attach-mixin'; -import { createStoreProviderMixin } from './provider-mixin'; +import { createContainerMixin } from './container-mixin'; +import { createProviderMixin } from './provider-mixin'; /** * Creates a combined mixin that both provides a store and auto-attaches media elements. @@ -29,13 +29,13 @@ export function createStoreMixin( context: Context, Features>>, factory: () => Store, Features> ): Mixin> { - const ProviderMixin = createStoreProviderMixin(context, factory); - const AttachMixin = createStoreAttachMixin(context); + const ProviderMixin = createProviderMixin(context, factory); + const ContainerMixin = createContainerMixin(context); return >(BaseClass: Base) => { // ProviderMixin wraps AttachMixin so during connectedCallback: // 1. ProviderMixin runs first (provides store via context) // 2. AttachMixin runs second (consumes store from context) - return ProviderMixin(AttachMixin(BaseClass)); + return ProviderMixin(ContainerMixin(BaseClass)); }; } diff --git a/packages/store/src/lit/mixins/attach-mixin.ts b/packages/store/src/lit/mixins/container-mixin.ts similarity index 93% rename from packages/store/src/lit/mixins/attach-mixin.ts rename to packages/store/src/lit/mixins/container-mixin.ts index 417ea295..41504fcc 100644 --- a/packages/store/src/lit/mixins/attach-mixin.ts +++ b/packages/store/src/lit/mixins/container-mixin.ts @@ -20,12 +20,12 @@ import type { Store, StoreConsumer } from '../../core/store'; * * @example * ```ts - * const { StoreAttachMixin } = createStore({ features: [playbackFeature] }); + * const { ContainerMixin } = createStore({ features: [playbackFeature] }); * - * class MyControls extends StoreAttachMixin(LitElement) {} + * class MyControls extends ContainerMixin(LitElement) {} * ``` */ -export function createStoreAttachMixin( +export function createContainerMixin( context: Context, Features>> ): Mixin> { type ConsumedStore = Store, Features>; diff --git a/packages/store/src/lit/mixins/index.ts b/packages/store/src/lit/mixins/index.ts index 3c874593..91319045 100644 --- a/packages/store/src/lit/mixins/index.ts +++ b/packages/store/src/lit/mixins/index.ts @@ -1,3 +1,3 @@ -export { createStoreAttachMixin } from './attach-mixin'; export { createStoreMixin } from './combined-mixin'; -export { createStoreProviderMixin } from './provider-mixin'; +export { createContainerMixin } from './container-mixin'; +export { createProviderMixin } from './provider-mixin'; diff --git a/packages/store/src/lit/mixins/provider-mixin.ts b/packages/store/src/lit/mixins/provider-mixin.ts index 3a23011a..354cde53 100644 --- a/packages/store/src/lit/mixins/provider-mixin.ts +++ b/packages/store/src/lit/mixins/provider-mixin.ts @@ -16,18 +16,18 @@ import type { Store, StoreProvider } from '../../core/store'; * * @example * ```ts - * const { StoreProviderMixin } = createStore({ + * const { ProviderMixin } = createStore({ * features: [playbackFeature] * }); * - * class MyPlayer extends StoreProviderMixin(LitElement) { + * class MyPlayer extends ProviderMixin(LitElement) { * render() { * return html``; * } * } * ``` */ -export function createStoreProviderMixin( +export function createProviderMixin( context: Context, Features>>, factory: () => Store, Features> ): >(BaseClass: Base) => Base & Constructor> { diff --git a/packages/store/src/lit/mixins/tests/attach-mixin.test.ts b/packages/store/src/lit/mixins/tests/container-mixin.test.ts similarity index 65% rename from packages/store/src/lit/mixins/tests/attach-mixin.test.ts rename to packages/store/src/lit/mixins/tests/container-mixin.test.ts index 75b63e10..979cf008 100644 --- a/packages/store/src/lit/mixins/tests/attach-mixin.test.ts +++ b/packages/store/src/lit/mixins/tests/container-mixin.test.ts @@ -4,12 +4,12 @@ import { createLitTestStore, setupDomCleanup, TestBaseElement, uniqueTag } from setupDomCleanup(); -describe('createStoreAttachMixin', () => { +describe('createContainerMixin', () => { it('exposes store property (initially null without context)', async () => { - const { StoreAttachMixin } = createLitTestStore(); - const tagName = uniqueTag('test-attach-standalone'); + const { ContainerMixin } = createLitTestStore(); + const tagName = uniqueTag('test-container-standalone'); - class TestElement extends StoreAttachMixin(TestBaseElement) {} + class TestElement extends ContainerMixin(TestBaseElement) {} customElements.define(tagName, TestElement); const el = document.createElement(tagName) as TestElement; @@ -21,9 +21,9 @@ describe('createStoreAttachMixin', () => { }); it('can be applied to TestBaseElement', () => { - const { StoreAttachMixin } = createLitTestStore(); + const { ContainerMixin } = createLitTestStore(); - class MixedElement extends StoreAttachMixin(TestBaseElement) {} + class MixedElement extends ContainerMixin(TestBaseElement) {} expect(MixedElement.prototype).toBeInstanceOf(TestBaseElement); }); diff --git a/packages/store/src/lit/mixins/tests/provider-mixin.test.ts b/packages/store/src/lit/mixins/tests/provider-mixin.test.ts index d2ec96ac..75e8a288 100644 --- a/packages/store/src/lit/mixins/tests/provider-mixin.test.ts +++ b/packages/store/src/lit/mixins/tests/provider-mixin.test.ts @@ -4,12 +4,12 @@ import { createLitTestStore, setupDomCleanup, TestBaseElement, uniqueTag } from setupDomCleanup(); -describe('createStoreProviderMixin', () => { +describe('createProviderMixin', () => { it('creates store lazily on first access', async () => { - const { StoreProviderMixin } = createLitTestStore(); + const { ProviderMixin } = createLitTestStore(); const tagName = uniqueTag('test-provider'); - class TestElement extends StoreProviderMixin(TestBaseElement) {} + class TestElement extends ProviderMixin(TestBaseElement) {} customElements.define(tagName, TestElement); const el = document.createElement(tagName) as TestElement; @@ -21,10 +21,10 @@ describe('createStoreProviderMixin', () => { }); it('reuses same store instance', async () => { - const { StoreProviderMixin } = createLitTestStore(); + const { ProviderMixin } = createLitTestStore(); const tagName = uniqueTag('test-provider-reuse'); - class TestElement extends StoreProviderMixin(TestBaseElement) {} + class TestElement extends ProviderMixin(TestBaseElement) {} customElements.define(tagName, TestElement); const el = document.createElement(tagName) as TestElement; @@ -38,10 +38,10 @@ describe('createStoreProviderMixin', () => { }); it('destroys owned store on disconnect', async () => { - const { StoreProviderMixin } = createLitTestStore(); + const { ProviderMixin } = createLitTestStore(); const tagName = uniqueTag('test-provider-destroy'); - class TestElement extends StoreProviderMixin(TestBaseElement) {} + class TestElement extends ProviderMixin(TestBaseElement) {} customElements.define(tagName, TestElement); const el = document.createElement(tagName) as TestElement; @@ -57,10 +57,10 @@ describe('createStoreProviderMixin', () => { }); it('allows setting custom store via setter', async () => { - const { StoreProviderMixin, create } = createLitTestStore(); + const { ProviderMixin, create } = createLitTestStore(); const tagName = uniqueTag('test-provider-setter'); - class TestElement extends StoreProviderMixin(TestBaseElement) {} + class TestElement extends ProviderMixin(TestBaseElement) {} customElements.define(tagName, TestElement); const el = document.createElement(tagName) as TestElement; @@ -74,10 +74,10 @@ describe('createStoreProviderMixin', () => { }); it('does not destroy externally provided store on disconnect', async () => { - const { StoreProviderMixin, create } = createLitTestStore(); + const { ProviderMixin, create } = createLitTestStore(); const tagName = uniqueTag('test-provider-external'); - class TestElement extends StoreProviderMixin(TestBaseElement) {} + class TestElement extends ProviderMixin(TestBaseElement) {} customElements.define(tagName, TestElement); const el = document.createElement(tagName) as TestElement; @@ -93,10 +93,10 @@ describe('createStoreProviderMixin', () => { }); it('destroys old owned store when setting new store', async () => { - const { StoreProviderMixin, create } = createLitTestStore(); + const { ProviderMixin, create } = createLitTestStore(); const tagName = uniqueTag('test-provider-replace'); - class TestElement extends StoreProviderMixin(TestBaseElement) {} + class TestElement extends ProviderMixin(TestBaseElement) {} customElements.define(tagName, TestElement); const el = document.createElement(tagName) as TestElement; diff --git a/packages/store/src/lit/mixins/tests/types.test.ts b/packages/store/src/lit/mixins/tests/types.test.ts index 16020e53..683e04aa 100644 --- a/packages/store/src/lit/mixins/tests/types.test.ts +++ b/packages/store/src/lit/mixins/tests/types.test.ts @@ -12,18 +12,18 @@ describe('mixin types', () => { expectTypeOf().toHaveProperty('store'); }); - it('storeProviderMixin adds store property', () => { - const { StoreProviderMixin } = createLitTestStore(); - const _MixedElement = StoreProviderMixin(TestBaseElement); + it('providerMixin adds store property', () => { + const { ProviderMixin } = createLitTestStore(); + const _MixedElement = ProviderMixin(TestBaseElement); type Instance = InstanceType; // Verify store property exists on the mixed type expectTypeOf().toHaveProperty('store'); }); - it('storeAttachMixin adds store property', () => { - const { StoreAttachMixin } = createLitTestStore(); - const _MixedElement = StoreAttachMixin(TestBaseElement); + it('containerMixin adds store property', () => { + const { ContainerMixin } = createLitTestStore(); + const _MixedElement = ContainerMixin(TestBaseElement); type Instance = InstanceType; // Verify store property exists on the mixed type diff --git a/packages/store/src/lit/tests/create-store.test.ts b/packages/store/src/lit/tests/create-store.test.ts index f7aaf13e..fabc2f33 100644 --- a/packages/store/src/lit/tests/create-store.test.ts +++ b/packages/store/src/lit/tests/create-store.test.ts @@ -76,26 +76,26 @@ describe('createStore', () => { expect(typeof StoreMixin).toBe('function'); }); - it('returns StoreProviderMixin', () => { - const { StoreProviderMixin } = createStore({ features: [audioFeature] }); + it('returns ProviderMixin', () => { + const { ProviderMixin } = createStore({ features: [audioFeature] }); - expect(typeof StoreProviderMixin).toBe('function'); + expect(typeof ProviderMixin).toBe('function'); }); - it('returns StoreAttachMixin', () => { - const { StoreAttachMixin } = createStore({ features: [audioFeature] }); + it('returns ContainerMixin', () => { + const { ContainerMixin } = createStore({ features: [audioFeature] }); - expect(typeof StoreAttachMixin).toBe('function'); + expect(typeof ContainerMixin).toBe('function'); }); it('mixins can be applied to TestBaseElement', () => { - const { StoreMixin, StoreProviderMixin, StoreAttachMixin } = createStore({ + const { StoreMixin, ProviderMixin, ContainerMixin } = createStore({ features: [audioFeature], }); const Mixed1 = StoreMixin(TestBaseElement); - const Mixed2 = StoreProviderMixin(TestBaseElement); - const Mixed3 = StoreAttachMixin(TestBaseElement); + const Mixed2 = ProviderMixin(TestBaseElement); + const Mixed3 = ContainerMixin(TestBaseElement); expect(Mixed1.prototype).toBeInstanceOf(TestBaseElement); expect(Mixed2.prototype).toBeInstanceOf(TestBaseElement); @@ -108,8 +108,8 @@ describe('createStore', () => { const result = createStore({ features: [audioFeature] }); expect(result).toHaveProperty('StoreMixin'); - expect(result).toHaveProperty('StoreProviderMixin'); - expect(result).toHaveProperty('StoreAttachMixin'); + expect(result).toHaveProperty('ProviderMixin'); + expect(result).toHaveProperty('ContainerMixin'); expect(result).toHaveProperty('context'); expect(result).toHaveProperty('create'); expect(result).toHaveProperty('StoreController'); diff --git a/packages/store/src/react/context.tsx b/packages/store/src/react/context.tsx index d9ca27ff..a900c9b1 100644 --- a/packages/store/src/react/context.tsx +++ b/packages/store/src/react/context.tsx @@ -24,15 +24,6 @@ export function useStoreContext(): AnyStore { return store; } -/** - * Internal hook to get parent store from context. - * Returns null if no parent Provider exists. - * Used by Provider to implement the `inherit` prop. - */ -export function useParentStore(): AnyStore | null { - return useContext(StoreContext); -} - /** * Internal provider component that wraps children with store context. */ diff --git a/packages/store/src/react/create-store.tsx b/packages/store/src/react/create-store.tsx index 5109cf33..744556d9 100644 --- a/packages/store/src/react/create-store.tsx +++ b/packages/store/src/react/create-store.tsx @@ -1,11 +1,12 @@ -import { isNull, isUndefined } from '@videojs/utils/predicate'; +import { isUndefined } from '@videojs/utils/predicate'; import type { FC, ReactNode } from 'react'; -import { useEffect, useMemo, useState, useSyncExternalStore } from 'react'; +import { useEffect, useState } from 'react'; import type { AnyFeature, UnionFeatureRequests, UnionFeatureState, UnionFeatureTarget } from '../core/feature'; import type { StoreConfig } from '../core/store'; import { Store } from '../core/store'; -import { StoreContextProvider, useParentStore, useStoreContext } from './context'; +import { StoreContextProvider, useStoreContext } from './context'; +import { useStore as useStoreBase } from './hooks/use-store'; // ---------------------------------------- // Types @@ -25,12 +26,6 @@ export interface ProviderProps { * The Provider will NOT destroy this store on unmount. */ store?: Store, Features>; - /** - * If true, inherits the store from a parent Provider context instead of creating a new one. - * Useful when wrapping a skin with your own Provider to add custom hooks. - * Defaults to false (isolated store). - */ - inherit?: boolean; } export type UseStoreResult = UnionFeatureState & @@ -83,29 +78,19 @@ export function createStore( /** * Provider component that manages store lifecycle. * - * Resolution order: - * 1. If `store` prop provided, uses that store (no cleanup on unmount) - * 2. If `inherit={true}` and parent store exists, uses parent store (no cleanup) - * 3. Otherwise, creates a new store and destroys it on unmount + * If `store` prop is provided, uses that store (no cleanup on unmount). + * Otherwise, creates a new store and destroys it on unmount. */ - function Provider({ children, store: providedStore, inherit = false }: ProviderProps): ReactNode { - const parentStore = useParentStore(); - const shouldInherit = inherit && !isNull(parentStore); - + function Provider({ children, store: providedStore }: ProviderProps): ReactNode { const [store] = useState(() => { if (!isUndefined(providedStore)) { return providedStore; } - if (shouldInherit) { - return parentStore as StoreType; - } - return create(); }); - // Only destroy if we created the store (not provided, not inherited) - const isOwner = isUndefined(providedStore) && !shouldInherit; + const isOwner = isUndefined(providedStore); useEffect(() => { if (isOwner) { @@ -125,17 +110,7 @@ export function createStore( function useStore(): UseStoreResult { const store = useStoreContext(); - - const state = useSyncExternalStore( - (cb) => store.subscribe(cb), - () => store.state, - () => store.state - ); - - return useMemo( - () => ({ ...state, ...(store.request as object) }) as UseStoreResult, - [state, store.request] - ); + return useStoreBase(store) as UseStoreResult; } return {