Issue #64 - order of molecule shouldn't matter

src/react/ScopeProvider.test.tsx

@@ -1,4 +1,4 @@
-import { act, renderHook } from "@testing-library/react";
+import { act, render, renderHook } from "@testing-library/react";
 import {
   Atom,
   PrimitiveAtom,
@@ -10,21 +10,34 @@ import {
 } from "jotai";
 import React, { ReactNode, useContext, useEffect } from "react";
 import { createLifecycleUtils } from "../shared/testing/lifecycle";
-import { createScope, molecule, resetDefaultInjector, use } from "../vanilla";
+import {
+  ComponentScope,
+  createScope,
+  molecule,
+  resetDefaultInjector,
+  use,
+} from "../vanilla";
 import { ScopeProvider } from "./ScopeProvider";
 import { ScopeContext } from "./contexts/ScopeContext";
 import { strictModeSuite } from "./testing/strictModeSuite";
 import { useMolecule } from "./useMolecule";
+import { AnyScopeTuple } from "../vanilla/internal/internal-types";
+import { useScopeTuplesRaw, useScopes } from "./useScopes";
+import { Debug } from "../vanilla/internal/symbols";
 
 const ExampleMolecule = molecule(() => {
   return {
     example: Math.random(),
   };
 });
 
-export const UserScope = createScope("[email protected]");
+export const UserScope = createScope("[email protected]", {
+  debugLabel: "UserScope",
+});
 
-const AtomScope = createScope(atom("[email protected]"));
+const AtomScope = createScope(atom("[email protected]"), {
+  debugLabel: "AtomScope",
+});
 
 const userLifecycle = createLifecycleUtils();
 export const UserMolecule = molecule(() => {
@@ -475,4 +488,103 @@ strictModeSuite(({ wrapper: Outer, isStrict }) => {
       userLifecycle.expectToMatchCalls(["[email protected]"]);
     }
   });
+
+  describe("Issue #64 - Peer providers don't share a value", () => {
+    const Wrapper = ({ children }: { children?: React.ReactNode }) => (
+      <Outer>{children}</Outer>
+    );
+
+    const Nested = molecule(() => use(UserMolecule));
+
+    const NestedComponent = () => (
+      <>
+        <div data-testid="nested">{useMolecule(Nested).example}</div>
+        <div data-testid="nested-scopes">
+          {useScopeTuplesRaw()
+            .filter((x) => x[0] !== ComponentScope)
+            .map(getTupleId)
+            .join(",")}
+        </div>
+      </>
+    );
+    const NonNestedComponent = () => (
+      <>
+        <div data-testid="non-nested">{useMolecule(UserMolecule).example}</div>
+        <div data-testid="non-nested-scopes">
+          {useScopeTuplesRaw()
+            .filter((x) => x[0] !== ComponentScope)
+            .map(getTupleId)
+            .join(",")}
+        </div>
+      </>
+    );
+
+    const tupleIds = new Map<AnyScopeTuple, string>();
+    const getTupleId = (tuple: AnyScopeTuple) => {
+      const found = tupleIds.get(tuple);
+      if (found) return found;
+      const id =
+        (tuple[0][Debug] as symbol)?.description +
+        "=" +
+        tuple[1] +
+        ";tuple=" +
+        Math.random();
+      tupleIds.set(tuple, id);
+      return id;
+    };
+
+    test.each([{ tcase: "nested" }, { tcase: "direct" }])(
+      "Should render when $tcase is first",
+      async ({ tcase }) => {
+        userLifecycle.expectUncalled();
+
+        const result = render(
+          tcase === "nested" ? (
+            <>
+              <ScopeProvider scope={UserScope} value="bob">
+                <NestedComponent />
+              </ScopeProvider>
+              <ScopeProvider scope={UserScope} value="bob">
+                <NonNestedComponent />
+              </ScopeProvider>
+            </>
+          ) : (
+            <>
+              <ScopeProvider scope={UserScope} value="bob">
+                <NonNestedComponent />
+              </ScopeProvider>
+              <ScopeProvider scope={UserScope} value="bob">
+                <NestedComponent />
+              </ScopeProvider>
+            </>
+          ),
+          {
+            wrapper: Wrapper,
+          },
+        );
+
+        // if (isStrict) {
+        //   userLifecycle.expectCalledTimesEach(1, 2, 1);
+        // } else {
+        //   userLifecycle.expectCalledTimesEach(1, 1, 0);
+        // }
+
+        const nestedScopes = await result.findByTestId("nested-scopes");
+        const nonNestedScopes = await result.findByTestId("non-nested-scopes");
+        expect(nestedScopes.innerText).toBe(nonNestedScopes.innerText);
+
+        const nestedValue = await result.findByTestId("nested");
+        const nonNestedValue = await result.findByTestId("non-nested");
+        expect(nestedValue.innerText).toBe(nonNestedValue.innerText);
+        result.unmount();
+
+        // if (isStrict) {
+        //   userLifecycle.expectCalledTimesEach(1, 2, 2);
+        // } else {
+        //   userLifecycle.expectCalledTimesEach(1, 1, 1);
+        //   userLifecycle.expectToMatchCalls(["bob"]);
+        // }
+      },
+    );
+  });
 });

src/react/useMolecule.tsx

@@ -35,7 +35,7 @@ export function useMolecule<T>(
     mol,
     injector,
     /**
-     * Tuple flattening prevents re-renders unless the number of
+     * Tuple flattening prevents re-renders unless the number of tuples changes
      */
     ...flattenTuples(inputTuples),
   ]);

src/shared/testing/lifecycle.ts

@@ -37,9 +37,14 @@ export function createLifecycleUtils() {
     timesMounted: number,
     timesUnmounted: number,
   ) => {
-    expect.soft(executions).toHaveBeenCalledTimes(timesExecuted);
-    expect.soft(mounts).toHaveBeenCalledTimes(timesMounted);
-    expect(unmounts).toHaveBeenCalledTimes(timesUnmounted);
+    expect(
+      [
+        executions.mock.calls.length,
+        mounts.mock.calls.length,
+        unmounts.mock.calls.length,
+      ],
+      "component lifecycle calls mismatch",
+    ).toStrictEqual([timesExecuted, timesMounted, timesUnmounted]);
   };
 
   const expectToHaveBeenCalledTimes = (num: number) => {

src/vanilla/injector.test.ts

@@ -119,6 +119,35 @@ describe("Scoping", () => {
     expect(thirdValue).not.toBe(secondValue);
   });
 
+  test.each([{ first: "direct" }, { first: "indirect" }])(
+    "Issue #64 - Works with both directions of scoping, starting with $first",
+    ({ first }) => {
+      const Direct = molecule(() => use(UserScope) + Math.random());
+      const Indirect = molecule(() => use(Direct));
+
+      const injector = createInjector();
+
+      const sub1 = injector.useLazily(first === "direct" ? Direct : Indirect, [
+        UserScope,
+        "bob",
+      ]);
+      const sub2 = injector.useLazily(first === "direct" ? Indirect : Direct, [
+        UserScope,
+        "bob",
+      ]);
+
+      expect(sub1[0]).not.toBe(sub2[0]);
+
+      const new1 = sub1[1].start();
+      const new2 = sub2[1].start();
+
+      expect(new1).toBe(new2);
+
+      sub1[1].stop();
+      sub2[1].stop();
+    },
+  );
+
   test("Creates only one molecule per dependent scope", () => {
     const injector = createInjector();
 

src/vanilla/injector.ts

@@ -263,8 +263,8 @@ export function createInjector(
         cachedDeps.has(tuple[0]),
       );
 
-      const deps = getCachePath(relevantScopes, m);
-      const cachedValue = moleculeCache.get(deps);
+      const cacheScopes = getCachePath(relevantScopes);
+      const cachedValue = moleculeCache.get([m, ...cacheScopes]);
 
       if (cachedValue) {
         // Extend the lease to include the any default scopes
@@ -287,36 +287,66 @@ export function createInjector(
     }
     injectorProps.instrumentation?.stage1CacheMiss();
     const { previous } = props;
-    if (previous !== false) {
+
+    if (previous !== false && previousIsCacheable(m, props)) {
       return moleculeCache.deepCache(
         () => previous,
         () => {},
-        previous.path,
+        [previous.self, ...previous.scopes],
       );
     }
     return runAndCache<T>(m, props);
   }
 
+  function previousIsCacheable(m: AnyMolecule, props: CreationProps) {
+    const { previous } = props;
+
+    // If no previous value, then nothing is cacheable
+    if (previous === false) return false;
+
+    /**
+     * All buddies are:
+     *  - exact matches from the cache
+     *  - missing in the cache
+     *
+     * are not:
+     *  - conflicting (not equal) to their cache value
+     */
+
+    const buddiesAllGood = previous.deps.buddies.every((buddy) => {
+      const currentCacheValue = getInternal(buddy, {
+        scopes: buddy.scopes,
+        lease: props.lease,
+        previous: buddy,
+      });
+      // FIXME: I think that this value should be reversed...
+      return currentCacheValue !== buddy;
+    });
+
+    return buddiesAllGood;
+  }
+
   function multiCache(
     mol: AnyMolecule,
     scopes: AnyScopeTuple[],
-    createFn: () => Omit<Omit<MoleculeCacheValue, "path">, "instanceId">,
+    createFn: () => Omit<Omit<MoleculeCacheValue, "scopes">, "instanceId">,
     foundFn: (found: MoleculeCacheValue) => void,
   ): MoleculeCacheValue | undefined {
-    const deps = getCachePath(scopes, mol);
+    const cacheScopes = getCachePath(scopes);
 
     const cached = moleculeCache.deepCache(
       () => {
         const innerCached = {
           ...createFn(),
-          path: deps,
+          scopes: cacheScopes,
+          self: mol,
           instanceId: instanceId(),
         };
 
         return innerCached;
       },
       foundFn,
-      deps,
+      [mol, ...cacheScopes],
     );
     return cached;
   }
@@ -398,6 +428,7 @@ export function createInjector(
           deps: mounted.deps,
           value: mounted.value,
           isMounted: false,
+          self: m,
         };
         injectorProps.instrumentation?.stage2CacheMiss(created);
         return created;
@@ -452,7 +483,7 @@ export function createInjector(
        * Without this repeated calls to `injector.use` would not create
        * new values, and would not run lifecycle hooks (mount, unmount).
        */
-      moleculeCache.remove(...mol.path);
+      moleculeCache.remove(mol.self, ...mol.scopes);
       mol.isMounted = false;
     });
 
@@ -472,9 +503,7 @@ export function createInjector(
      * These are the scopes that were implicitly provided when the molecule
      * was created
      */
-    const usedScopes = mol.path.filter((molOrScope) =>
-      Array.isArray(molOrScope),
-    ) as AnyScopeTuple[];
+    const usedScopes = mol.scopes;
     scoper.registerCleanups(usedScopes, cleanupSet);
 
     injectorProps?.instrumentation?.mounted(mol, usedScopes, cleanupSet);
@@ -526,6 +555,17 @@ export function createInjector(
       }
       injectorProps?.instrumentation?.subscribe(bound, cacheValue);
 
+      /**
+       * TODO: Fix for #64
+       *
+       * We need to recursively check that not only this molecule is using
+       * the right value, but also that it's using the latest version of it's
+       * dependencies, too.
+       *
+       *
+       * Side effects:
+       * - more molecule executions, so will invalidate some tests that assume only 1 execution
+       */
       cacheValue = getInternal<T>(bound, {
         scopes: sub.start(),
         lease,
@@ -573,7 +613,7 @@ enum MoleculeSubscriptionState {
  * @param mol
  * @returns
  */
-function getCachePath(scopes: AnyScopeTuple[], mol: AnyMolecule) {
+function getCachePath(scopes: AnyScopeTuple[]) {
   /**
    * Important: We filter out default scopes as a part of the cache path
    * because it makes it easier for us to find a molecule in our Stage 1
@@ -585,8 +625,7 @@ function getCachePath(scopes: AnyScopeTuple[], mol: AnyMolecule) {
    * Important: Sorting of scopes is important to ensure a consistent path
    * for storing (and finding) molecules in the deep cache tree
    */
-  const deps = [mol, ...scopeTupleSort(nonDefaultScopes)];
-  return deps;
+  return scopeTupleSort(nonDefaultScopes);
 }
 
 /**

src/vanilla/internal/internal-types.ts

@@ -20,7 +20,8 @@ export type MoleculeCacheValue = {
   deps: Deps;
   value: unknown;
   isMounted: boolean;
-  path: (AnyScopeTuple | AnyMolecule)[];
+  scopes: AnyScopeTuple[];
+  self: AnyMolecule;
   instanceId: symbol;
 };