Skip to content

Commit

Permalink
fix: multislot views memoizing incorrectly
Browse files Browse the repository at this point in the history
Who would have thought those `as any` casts were masking something :thinking_face:

The cachekey for any multislot view was always the same so the first view was the only one that could ever exist

This obviously lead to problems if another view had to be created or the same one recreated (as it had a reference to a now stopped interpreter)
  • Loading branch information
UberMouse committed May 24, 2023
1 parent 5ab563a commit 791eeda
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 7 deletions.
30 changes: 28 additions & 2 deletions src/xstateTree.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { render } from "@testing-library/react";
import { assign } from "@xstate/immer";
import React from "react";
import { createMachine } from "xstate";
import { createMachine, interpret } from "xstate";

import {
buildXStateTreeMachine,
Expand All @@ -12,7 +12,11 @@ import {
} from "./builders";
import { singleSlot } from "./slots";
import { delay } from "./utils";
import { broadcast, buildRootComponent } from "./xstateTree";
import {
broadcast,
buildRootComponent,
getMultiSlotViewForChildren,
} from "./xstateTree";

describe("xstate-tree", () => {
describe("a machine with a guarded event that triggers external side effects in an action", () => {
Expand Down Expand Up @@ -236,4 +240,26 @@ describe("xstate-tree", () => {
} catch {}
expect(childMachineHandler).toHaveBeenCalled();
});

describe("getMultiSlotViewForChildren", () => {
it("memoizes correctly", () => {
const machine = createMachine({
id: "test",
initial: "idle",
states: {
idle: {},
},
});

const interpreter1 = interpret(machine).start();
const interpreter2 = interpret(machine).start();

const view1 = getMultiSlotViewForChildren(interpreter1, "ignored");
const view2 = getMultiSlotViewForChildren(interpreter2, "ignored");

expect(view1).not.toBe(view2);
expect(view1).toBe(getMultiSlotViewForChildren(interpreter1, "ignored"));
expect(view2).toBe(getMultiSlotViewForChildren(interpreter2, "ignored"));
});
});
});
11 changes: 6 additions & 5 deletions src/xstateTree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ function cacheKeyForInterpreter(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
interpreter: Interpreter<any, any, any>
) {
console.log("cacheKeyForInterpreter", interpreter);
return interpreter.sessionId;
}

Expand All @@ -91,7 +92,10 @@ const getViewForInterpreter = memoize(
{ serializer: cacheKeyForInterpreter as any }
);

const getMultiSlotViewForChildren = memoize(
/**
* @private
*/
export const getMultiSlotViewForChildren = memoize(
(parent: InterpreterFrom<AnyXstateTreeMachine>, slot: string) => {
return React.memo(function MultiSlotView() {
const [_, children] = useService(parent);
Expand All @@ -108,10 +112,7 @@ const getMultiSlotViewForChildren = memoize(
});
},
{
// eslint-disable-next-line @typescript-eslint/no-explicit-any
serializer: ((interpreter: any, slot: any) =>
// eslint-disable-next-line @typescript-eslint/no-explicit-any
`${cacheKeyForInterpreter(interpreter)}-${slot}`) as any,
serializer: (args) => `${cacheKeyForInterpreter(args[0])}-${args[1]}`,
}
);

Expand Down

0 comments on commit 791eeda

Please sign in to comment.