diff --git a/sandpack-client/src/iframe-protocol.ts b/sandpack-client/src/iframe-protocol.ts index cbbfaa3aa..9f79d4d0f 100644 --- a/sandpack-client/src/iframe-protocol.ts +++ b/sandpack-client/src/iframe-protocol.ts @@ -13,7 +13,7 @@ export class IFrameProtocol { private globalListenersCount = 0; // React to messages from the iframe owned by this instance - private channelListeners: Record = {}; + public channelListeners: Record = {}; private channelListenersCount = 0; // Random number to identify this instance of the client when messages are coming from multiple iframes diff --git a/sandpack-react/src/contexts/sandpackContext.test.tsx b/sandpack-react/src/contexts/sandpackContext.test.tsx index 591f6ef3b..515de2d78 100644 --- a/sandpack-react/src/contexts/sandpackContext.test.tsx +++ b/sandpack-react/src/contexts/sandpackContext.test.tsx @@ -1,15 +1,39 @@ +/** + * @jest-environment jsdom + */ import React from "react"; import { create } from "react-test-renderer"; import { REACT_TEMPLATE } from ".."; +import type { SandpackProviderClass } from "./sandpackContext"; import { SandpackProvider } from "./sandpackContext"; +const createContext = (): SandpackProviderClass => { + const root = create(); + const instance = root.getInstance(); + + instance.runSandpack(); + + return instance; +}; + +const getAmountOfListener = ( + instance: SandpackProviderClass, + name = "client-id", + ignoreGlobalListener = false +): number => { + return ( + Object.keys(instance.clients[name].iframeProtocol.channelListeners).length - + 1 - // less protocol listener + (ignoreGlobalListener ? 0 : 1) // less the global Sandpack-react listener + ); +}; + describe(SandpackProvider, () => { describe("updateFile", () => { it("adds a file", () => { - const root = create(); - const instance = root.getInstance(); + const instance = createContext(); instance.addFile({ "new-file.js": "new-content" }); @@ -17,8 +41,7 @@ describe(SandpackProvider, () => { }); it("deletes a file", () => { - const root = create(); - const instance = root.getInstance(); + const instance = createContext(); instance.deleteFile("/App.js"); @@ -32,8 +55,7 @@ describe(SandpackProvider, () => { }); it("updates a file", () => { - const root = create(); - const instance = root.getInstance(); + const instance = createContext(); expect(instance.state.files["/App.js"]).toEqual({ code: `export default function App() { @@ -48,8 +70,7 @@ describe(SandpackProvider, () => { }); it("updates multiples files", () => { - const root = create(); - const instance = root.getInstance(); + const instance = createContext(); instance.updateFile({ "/App.js": "Foo", "/index.js": "Baz" }); @@ -58,8 +79,7 @@ describe(SandpackProvider, () => { }); it("updates multiples files in a row", () => { - const root = create(); - const instance = root.getInstance(); + const instance = createContext(); instance.updateFile("/App.js", "Foo"); instance.updateFile("/index.js", "Baz"); @@ -71,15 +91,13 @@ describe(SandpackProvider, () => { describe("editorState", () => { it("should return the same initial state", () => { - const root = create(); - const instance = root.getInstance(); + const instance = createContext(); expect(instance.state.editorState).toBe("pristine"); }); it("should return a dirty value after updating a file", () => { - const root = create(); - const instance = root.getInstance(); + const instance = createContext(); expect(instance.state.editorState).toBe("pristine"); @@ -88,8 +106,7 @@ describe(SandpackProvider, () => { }); it("should return a pristine value after reseting files", () => { - const root = create(); - const instance = root.getInstance(); + const instance = createContext(); expect(instance.state.editorState).toBe("pristine"); @@ -101,8 +118,7 @@ describe(SandpackProvider, () => { }); it("should return a pristine value after reverting a change", () => { - const root = create(); - const instance = root.getInstance(); + const instance = createContext(); expect(instance.state.editorState).toBe("pristine"); instance.updateFile("/App.js", "Foo"); @@ -113,4 +129,208 @@ describe(SandpackProvider, () => { expect(instance.state.editorState).toBe("pristine"); }); }); + + describe("listeners", () => { + it("sets a listener, but the client hasn't been created yet - no global listener", () => { + const instance = createContext(); + + // Act: Add listener + const mock = jest.fn(); + instance.addListener(mock, "client-id"); + + // Act: Create client + instance.registerBundler(document.createElement("iframe"), "client-id"); + + // Expect: one pending unsubscribe function + expect( + Object.keys(instance.unsubscribeClientListeners["client-id"]).length + ).toBe(1); + + // Expect: no global listener + expect(Object.keys(instance.queuedListeners.global).length).toBe(0); + + // Expect: one client + expect(Object.keys(instance.clients)).toEqual(["client-id"]); + + /** + * TODO: figure out how to mock SandpackClient and invoke the listener func + */ + // expect(mock).toHaveBeenCalled(); + }); + + it("sets a listener, but the client hasn't been created yet - global listener", () => { + const instance = createContext(); + + // Act: Add listener + const mock = jest.fn(); + instance.addListener(mock /* , no client-id */); + + // Act: Create client + instance.registerBundler(document.createElement("iframe"), "client-id"); + + // Expect: one pending unsubscribe function + expect( + Object.keys(instance.unsubscribeClientListeners["client-id"]).length + ).toBe(1); + + // Expect: no global listener + expect(Object.keys(instance.queuedListeners.global).length).toBe(1); + + // Expect: one listener in the client + expect(getAmountOfListener(instance)).toBe(1); + }); + + it("set a listener, but the client has already been created - no global listener", () => { + const instance = createContext(); + + // Act: Create client + instance.registerBundler(document.createElement("iframe"), "client-id"); + + // Expect: no pending unsubscribe function + expect( + Object.keys(instance.unsubscribeClientListeners["client-id"]).length + ).toBe(0); + + // Expect: no global listener + expect(Object.keys(instance.queuedListeners.global).length).toBe(0); + + // Act: Add listener + const mock = jest.fn(); + instance.addListener(mock, "client-id"); + + // Expect: no pending unsubscribe function + expect( + Object.keys(instance.unsubscribeClientListeners["client-id"]).length + ).toBe(0); + + // Expect: no global listener + expect(Object.keys(instance.queuedListeners.global).length).toBe(0); + + // Expect: one listener in the client + expect(getAmountOfListener(instance)).toBe(1); + }); + + it("set a listener, but the client has already been created - global listener", () => { + const instance = createContext(); + + // Act: Create client + instance.registerBundler(document.createElement("iframe"), "client-id"); + + // Expect: no pending unsubscribe function + expect( + Object.keys(instance.unsubscribeClientListeners["client-id"]).length + ).toBe(0); + + // Expect: no global listener + expect(Object.keys(instance.queuedListeners.global).length).toBe(0); + + // Act: Add listener + const mock = jest.fn(); + instance.addListener(mock /* , no client-id */); + + // Expect: no pending unsubscribe function, because it's a global + expect( + Object.keys(instance.unsubscribeClientListeners["client-id"]).length + ).toBe(0); + + // Expect: one global listener + expect(Object.keys(instance.queuedListeners.global).length).toBe(1); + + // Expect: one listener in the client + expect(getAmountOfListener(instance)).toBe(1); + }); + + it("sets a new listener, and then create one more client", () => { + const instance = createContext(); + + // Act: Add listener + const mock = jest.fn(); + instance.addListener(mock, "client-id"); + + // Act: Create client + instance.registerBundler(document.createElement("iframe"), "client-id"); + + // Expect: one pending unsubscribe function + expect( + Object.keys(instance.unsubscribeClientListeners["client-id"]).length + ).toBe(1); + + // Expect: no global listener + expect(Object.keys(instance.queuedListeners.global).length).toBe(0); + + // Expect: one listener in the client + expect(getAmountOfListener(instance)).toBe(1); + + // Act: Add one more listener + const anotherMock = jest.fn(); + instance.addListener(anotherMock /* , no client-id */); + + // Expect: one global listener + expect(Object.keys(instance.queuedListeners.global).length).toBe(1); + + // Expect: two listener in the client + expect(getAmountOfListener(instance)).toBe(2); + }); + + it("unsubscribes only from the assigned client id", () => { + const instance = createContext(); + + instance.registerBundler(document.createElement("iframe"), "client-1"); + instance.registerBundler(document.createElement("iframe"), "client-2"); + + // Initial state + expect(getAmountOfListener(instance, "client-1")).toBe(0); + expect(getAmountOfListener(instance, "client-2", true)).toBe(0); + + // Add listeners + instance.addListener(jest.fn(), "client-1"); + const unsubscribeClientTwo = instance.addListener(jest.fn(), "client-2"); + + expect(getAmountOfListener(instance, "client-1")).toBe(1); + expect(getAmountOfListener(instance, "client-2", true)).toBe(1); + + unsubscribeClientTwo(); + + expect(getAmountOfListener(instance, "client-1")).toBe(1); + expect(getAmountOfListener(instance, "client-2", true)).toBe(0); + }); + + it("doesn't trigger global unsubscribe", () => { + const instance = createContext(); + + instance.registerBundler(document.createElement("iframe"), "client-1"); + instance.registerBundler(document.createElement("iframe"), "client-2"); + + instance.addListener(jest.fn()); + instance.addListener(jest.fn()); + const unsubscribe = instance.addListener(jest.fn()); + + expect(getAmountOfListener(instance, "client-1")).toBe(3); + expect(getAmountOfListener(instance, "client-2", true)).toBe(3); + + unsubscribe(); + + expect(getAmountOfListener(instance, "client-1")).toBe(2); + expect(getAmountOfListener(instance, "client-2", true)).toBe(2); + }); + + it("unsubscribe all the listeners from a specific client when it unmonts", () => { + const instance = createContext(); + + instance.registerBundler(document.createElement("iframe"), "client-1"); + instance.registerBundler(document.createElement("iframe"), "client-2"); + + instance.addListener(jest.fn()); + instance.addListener(jest.fn()); + instance.addListener(jest.fn()); + + expect(getAmountOfListener(instance, "client-1")).toBe(3); + expect(getAmountOfListener(instance, "client-2", true)).toBe(3); + + instance.unregisterBundler("client-2"); + + expect(getAmountOfListener(instance, "client-1")).toBe(3); + expect(instance.clients["client-2"]).toBe(undefined); + }); + }); }); diff --git a/sandpack-react/src/contexts/sandpackContext.tsx b/sandpack-react/src/contexts/sandpackContext.tsx index 3f5934e22..146418337 100644 --- a/sandpack-react/src/contexts/sandpackContext.tsx +++ b/sandpack-react/src/contexts/sandpackContext.tsx @@ -1,7 +1,6 @@ import { ClasserProvider } from "@code-hike/classer"; import type { ListenerFunction, - SandpackBundlerFiles, SandpackMessage, UnsubscribeFunction, ReactDevToolsMode, @@ -40,7 +39,7 @@ const BUNDLER_TIMEOUT = 30000; // 30 seconds timeout for the bundler to respond. * @category Provider * @hidden */ -class SandpackProviderClass extends React.PureComponent< +export class SandpackProviderClass extends React.PureComponent< SandpackProviderProps, SandpackProviderState > { @@ -55,7 +54,7 @@ class SandpackProviderClass extends React.PureComponent< intersectionObserver?: IntersectionObserver; queuedListeners: Record>; - unsubscribeQueuedListeners: Record< + unsubscribeClientListeners: Record< string, Record >; @@ -98,7 +97,7 @@ class SandpackProviderClass extends React.PureComponent< /** * Global list of unsubscribe function for the listeners */ - this.unsubscribeQueuedListeners = {}; + this.unsubscribeClientListeners = {}; this.preregisteredIframes = {}; this.clients = {}; @@ -369,6 +368,9 @@ class SandpackProviderClass extends React.PureComponent< }, BUNDLER_TIMEOUT); } + this.unsubscribeClientListeners[clientId] = + this.unsubscribeClientListeners[clientId] || {}; + /** * Register any potential listeners that subscribed before sandpack ran */ @@ -376,7 +378,7 @@ class SandpackProviderClass extends React.PureComponent< Object.keys(this.queuedListeners[clientId]).forEach((listenerId) => { const listener = this.queuedListeners[clientId][listenerId]; const unsubscribe = client.listen(listener) as () => void; - this.unsubscribeQueuedListeners[clientId][listenerId] = unsubscribe; + this.unsubscribeClientListeners[clientId][listenerId] = unsubscribe; }); // Clear the queued listeners after they were registered @@ -389,7 +391,7 @@ class SandpackProviderClass extends React.PureComponent< const globalListeners = Object.entries(this.queuedListeners.global); globalListeners.forEach(([listenerId, listener]) => { const unsubscribe = client.listen(listener) as () => void; - this.unsubscribeQueuedListeners[clientId][listenerId] = unsubscribe; + this.unsubscribeClientListeners[clientId][listenerId] = unsubscribe; /** * Important: Do not clean the global queue @@ -432,6 +434,16 @@ class SandpackProviderClass extends React.PureComponent< clearTimeout(this.timeoutHook); } + const unsubscribeQueuedClients = Object.values( + this.unsubscribeClientListeners + ); + + // Unsubscribing all listener registered + unsubscribeQueuedClients.forEach((listenerOfClient) => { + const listenerFunctions = Object.values(listenerOfClient); + listenerFunctions.forEach((unsubscribe) => unsubscribe()); + }); + this.setState({ sandpackStatus: "idle" }); }; @@ -523,26 +535,32 @@ class SandpackProviderClass extends React.PureComponent< return unsubscribeListener; } else { - // When listeners are added before the client is instantiated, they are stored with an unique id - // When the client is eventually instantiated, the listeners are registered on the spot - // Their unsubscribe functions are stored in unsubscribeQueuedListeners for future cleanup + /** + * When listeners are added before the client is instantiated, they are stored with an unique id + * When the client is eventually instantiated, the listeners are registered on the spot + * Their unsubscribe functions are stored in unsubscribeClientListeners for future cleanup + */ const listenerId = generateRandomId(); this.queuedListeners[clientId] = this.queuedListeners[clientId] || {}; - this.unsubscribeQueuedListeners[clientId] = - this.unsubscribeQueuedListeners[clientId] || {}; + this.unsubscribeClientListeners[clientId] = + this.unsubscribeClientListeners[clientId] || {}; this.queuedListeners[clientId][listenerId] = listener; const unsubscribeListener = (): void => { if (this.queuedListeners[clientId][listenerId]) { - // unsubscribe was called before the client was instantiated - // common example - a component with autorun=false that unmounted + /** + * Unsubscribe was called before the client was instantiated + * common example - a component with autorun=false that unmounted + */ delete this.queuedListeners[clientId][listenerId]; - } else if (this.unsubscribeQueuedListeners[clientId][listenerId]) { - // unsubscribe was called for a listener that got added before the client was instantiated - // call the unsubscribe function and remove it from memory - this.unsubscribeQueuedListeners[clientId][listenerId](); - delete this.unsubscribeQueuedListeners[clientId][listenerId]; + } else if (this.unsubscribeClientListeners[clientId][listenerId]) { + /** + * unsubscribe was called for a listener that got added before the client was instantiated + * call the unsubscribe function and remove it from memory + */ + this.unsubscribeClientListeners[clientId][listenerId](); + delete this.unsubscribeClientListeners[clientId][listenerId]; } }; @@ -560,16 +578,6 @@ class SandpackProviderClass extends React.PureComponent< ); const unsubscribeListener = (): void => { - const unsubscribeQueuedClients = Object.values( - this.unsubscribeQueuedListeners - ); - - // Unsubscribing all listener registered - unsubscribeQueuedClients.forEach((listenerOfClient) => { - const listenerFunctions = Object.values(listenerOfClient); - listenerFunctions.forEach((unsubscribe) => unsubscribe()); - }); - // Unsubscribing from the clients already created currentClientUnsubscribeListeners.forEach((unsubscribe) => unsubscribe()