fix(#447): Fix contexts implementation

doug-wade · Dec 6, 2023 · 5050082 · 5050082
1 parent 5f4ee88
commit 5050082
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 65 deletions.
diff --git a/packages/core/src/api/context-event.ts b/packages/core/src/api/context-event.ts
@@ -1,6 +1,6 @@
 import type { Context, ContextCallback, ContextEvent, ContextType, UnknownContext } from '../types';
 
-class TybaltContextEvent<T> implements ContextEvent<T extends UnknownContext ? any : any> {
+class TybaltContextEvent<T> extends Event implements ContextEvent<T extends UnknownContext ? any : any> {
     #context: Context<T>;
     #callback: ContextCallback<ContextType<T extends UnknownContext ? any : any>>;
     #subscribe: boolean;
@@ -26,9 +26,11 @@ class TybaltContextEvent<T> implements ContextEvent<T extends UnknownContext ? a
             type?: string;
         },
     ) {
+        super('context-request');
+
         this.#context = context;
         this.#callback = callback;
-        this.#subscribe = options.subscribe || false;
+        this.#subscribe = options?.subscribe || false;
         this.#event = new CustomEvent('context-request', options);
     }
 

diff --git a/packages/core/src/api/define-component.ts b/packages/core/src/api/define-component.ts
@@ -62,7 +62,7 @@ export default ({
 
         // All of the contexts to connect to
         // https://github.com/webcomponents-cg/community-protocols/blob/main/proposals/context.md
-        #contexts = new Map();
+        #contexts = new Map<string, { value: any, observable: BehaviorSubject<any>, unsubscribe?: () => void }>();
         contextState: any;
 
         constructor() {
@@ -102,6 +102,48 @@ export default ({
                 },
             );
 
+            for (const [contextName, context] of Object.entries(contexts)) {
+                const observable = new BehaviorSubject(context.initialValue || null);
+                this.#contexts.set(contextName, { value: context.initialValue, observable });
+
+                this.dispatchEvent(
+                    new ContextEvent(
+                        context,
+                        (value, unsubscribe) => {
+                            const contextState = this.#contexts.get(context) || { value: undefined, unsubscribe: undefined };
+
+                            // Call the old unsubscribe callback if the unsubscribe call has
+                            // changed. This probably means we have a new provider.
+                            if (unsubscribe !== contextState.unsubscribe) {
+                                contextState.unsubscribe?.();
+                            }
+
+                            observable.next(value);
+
+                            this.#contexts.set(contextName, { value, unsubscribe, observable });
+                        },
+                        {
+                            subscribe: true,
+                        },
+                    ),
+                );
+
+                /**
+                 * We want to make prop values and contexts available in the render function without needing to
+                 * pass them in the setup function. We don't want to shadow them if the dev wants to
+                 * reuse derived state with the same name in their render function or template.
+                 */
+                if (!this.#props[contextName]) {
+                    this.#renderObservables[contextName] = observable;
+                } else {
+                    /** 
+                     * DBW 12/6/23: I would prefer to throw here, but I can't catch the error and I can catch the log line
+                     * in the unit tests, so we log because its what we can test 🤣.
+                     */
+                    console.warn(`Collision detected between context and prop: ${contextName}`);
+                }
+            }
+
             // This is the method for clients to use to emit events
             const emit = (type: string, detail: any) => {
                 if (emits && !emits?.includes(type)) {
@@ -114,7 +156,7 @@ export default ({
                 emit,
             };
 
-            const getProxy = (value: { observable: BehaviorSubject<any>; parser: { parse(str: string | null): any; }; }) => {
+            const getProxy = (value: { observable: BehaviorSubject<any>; parser?: { parse(str: string | null): any; }; unsubscribe?: () => void }) => {
                 return new Proxy(value, {
                     get(target, prop, receiver) {
                         if (prop === 'value') {
@@ -125,9 +167,11 @@ export default ({
                     }
                 });
             };
-            const propsForSetup = Object.fromEntries(
-                Object.entries(this.#props).map(([key, value]) => [key, getProxy(value)]),
-            )
+
+            const propsForSetup: { [key: string]: { subscribe: () => void; observable: Observable<any> } } = Object.fromEntries([
+                ...Object.entries(this.#props).map(([key, value]) => [key, getProxy(value)]),
+                ...Array.from(this.#contexts.entries()).map(([key, value]) => { return [key, getProxy(value)] }),
+            ]);
 
             const setupResults =
                 setup?.call(
@@ -148,45 +192,6 @@ export default ({
                 }
             }
 
-            for (const context of contexts) {
-                this.#contexts.set(context, { value: {}, unsubscribe: () => {} });
-
-                const observable = new BehaviorSubject(context.initialValue || null);
-
-                this.dispatchEvent(
-                    new ContextEvent(
-                        context,
-                        (value, unsubscribe) => {
-                            const contextState = this.#contexts.get(context);
-
-                            // Call the old unsubscribe callback if the unsubscribe call has
-                            // changed. This probably means we have a new provider.
-                            if (unsubscribe !== contextState.unsubscribe) {
-                                contextState.unsubscribe?.();
-                            }
-
-                            observable.next(value);
-
-                            this.contextState.set(context, { value, unsubscribe, observable });
-                        },
-                        {
-                            subscribe: true,
-                        },
-                    ),
-                );
-            }
-
-            /**
-             * We want to make prop values and contexts available in the render function without needing to
-             * pass them in the setup function. We don't want to shadow them if the dev wants to
-             * reuse derived state with the same name in their render function or template.
-             */
-            for (const [key, value] of Object.entries(this.#contexts)) {
-                if (!this.#renderObservables[key]) {
-                    this.#renderObservables[key] = value.observable;
-                }
-            }
-
             for (const [key, value] of Object.entries(this.#props)) {
                 if (!this.#renderObservables[key]) {
                     // dbw 7/29/23: We convert the BehaviorSubject to an Observable here because its easier to use

diff --git a/packages/core/src/types.d.ts b/packages/core/src/types.d.ts
@@ -30,7 +30,7 @@ export type DefineComponentsOptions = {
     shadowMode?: 'open' | 'closed';
     css?: string | ((RenderContext) => string);
     template?: string;
-    contexts?: Context[];
+    contexts?: { [key: string]: Context };
 };
 
 export type UseObservableOptions =

diff --git a/packages/core/tst/integration/contexts.test.ts b/packages/core/tst/integration/contexts.test.ts
@@ -11,31 +11,29 @@ describe('contexts', () => {
         let actual: any = {};
         const component = defineComponent({
             name: 'passes-contexts-to-setup',
-            setup({ context }) {
-                console.log('context:', context);
-                actual = context;
+            setup({ example }) {
+                actual = example;
             },
-            contexts: { context },
+            contexts: { example: context },
         });
 
         await mount(component);
 
-        console.log('actual:', actual);
-
-        expect(actual.name).toStrictEqual(mockContextName);
-        expect(actual.initialValue).toStrictEqual(mockContextValue);
+        expect(actual.value).toStrictEqual(mockContextValue);
     });
 
-    it('throws an error if there is a collision', async () => {
+    it('emits a warning if there is a collision', async () => {
+        const jestSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});
         const context = createContext('bmo mochi');
-        expect(() => {
-            defineComponent({
-                name: 'passes-contexts-to-setup',
-                props: {
-                    example: { default: 'hello world' }
-                },
-                contexts: { example: context },
-            });
-        }).toThrow('Collision detected between context and prop: example')
+
+        await mount(defineComponent({
+            name: 'throws-an-error-if-there-is-a-collision',
+            props: {
+                example: { default: 'hello world' }
+            },
+            contexts: { example: context },
+        }));
+
+        expect(jestSpy.mock.calls[0][0]).toBe('Collision detected between context and prop: example');
     });
 });