From 00eec3d58000a272e3b27f91c50188815a531f19 Mon Sep 17 00:00:00 2001 From: Taylor Lodge Date: Wed, 9 Aug 2023 14:46:34 +1200 Subject: [PATCH] fix(routing): notify about double routing roots Having two routing roots on the page at once causes problems, as they are both listening to the same routing events If they have the same routes supplied it's probably ok, just double broadcast messages. But if they have different routes then one will broadcast a 404 event, and one will not. Better not to do it at all. So that will now throw an error in dev and log an error message in prod to enable it to be caught --- package-lock.json | 13 ++++--- package.json | 5 ++- src/routing/Link.tsx | 2 +- src/routing/index.ts | 6 ++- src/routing/providers.tsx | 9 +++++ src/xstateTree.spec.tsx | 78 +++++++++++++++++++++++++++++++++++++++ src/xstateTree.tsx | 11 ++++++ tsconfig.json | 2 +- 8 files changed, 115 insertions(+), 11 deletions(-) diff --git a/package-lock.json b/package-lock.json index 6280aa6..f3e55f4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -26,6 +26,7 @@ "@testing-library/user-event": "^14.4.3", "@types/history": "^4.7.7", "@types/jest": "^28.1.4", + "@types/node": "^20.4.9", "@types/react": "^17.0.29", "@types/react-dom": "^18.0.6", "@types/testing-library__jest-dom": "^5.14.1", @@ -3060,9 +3061,9 @@ "dev": true }, "node_modules/@types/node": { - "version": "18.0.3", - "resolved": "https://registry.npmjs.org/@types/node/-/node-18.0.3.tgz", - "integrity": "sha512-HzNRZtp4eepNitP+BD6k2L6DROIDG4Q0fm4x+dwfsr6LGmROENnok75VGw40628xf+iR24WeMFcHuuBDUAzzsQ==", + "version": "20.4.9", + "resolved": "https://registry.npmjs.org/@types/node/-/node-20.4.9.tgz", + "integrity": "sha512-8e2HYcg7ohnTUbHk8focoklEQYvemQmu9M/f43DZVx43kHn0tE3BY/6gSDxS7k0SprtS0NHvj+L80cGLnoOUcQ==", "dev": true }, "node_modules/@types/normalize-package-data": { @@ -20876,9 +20877,9 @@ "dev": true }, "@types/node": { - "version": "18.0.3", - "resolved": "https://registry.npmjs.org/@types/node/-/node-18.0.3.tgz", - "integrity": "sha512-HzNRZtp4eepNitP+BD6k2L6DROIDG4Q0fm4x+dwfsr6LGmROENnok75VGw40628xf+iR24WeMFcHuuBDUAzzsQ==", + "version": "20.4.9", + "resolved": "https://registry.npmjs.org/@types/node/-/node-20.4.9.tgz", + "integrity": "sha512-8e2HYcg7ohnTUbHk8focoklEQYvemQmu9M/f43DZVx43kHn0tE3BY/6gSDxS7k0SprtS0NHvj+L80cGLnoOUcQ==", "dev": true }, "@types/normalize-package-data": { diff --git a/package.json b/package.json index 9b73e29..33c2788 100644 --- a/package.json +++ b/package.json @@ -37,6 +37,7 @@ "@testing-library/user-event": "^14.4.3", "@types/history": "^4.7.7", "@types/jest": "^28.1.4", + "@types/node": "^20.4.9", "@types/react": "^17.0.29", "@types/react-dom": "^18.0.6", "@types/testing-library__jest-dom": "^5.14.1", @@ -72,9 +73,9 @@ }, "peerDependencies": { "@xstate/react": "^3.x", + "react": ">= 16.8.0 < 19.0.0", "xstate": ">= 4.20 < 5.0.0", - "zod": "^3.x", - "react": ">= 16.8.0 < 19.0.0" + "zod": "^3.x" }, "scripts": { "lint": "eslint 'src/**/*'", diff --git a/src/routing/Link.tsx b/src/routing/Link.tsx index 40fe71e..5ee8621 100644 --- a/src/routing/Link.tsx +++ b/src/routing/Link.tsx @@ -61,7 +61,7 @@ export function Link({ // and everything that consumes params/query already checks for undefined const { params, query, meta, ...props } = rest; - let timeout: number | undefined; + let timeout: ReturnType | undefined; const href = useHref(to, params, query); const onMouseDown: React.MouseEventHandler | undefined = preloadOnInteraction diff --git a/src/routing/index.ts b/src/routing/index.ts index da775c1..59c31da 100644 --- a/src/routing/index.ts +++ b/src/routing/index.ts @@ -23,4 +23,8 @@ export { export { useIsRouteActive } from "./useIsRouteActive"; export { useRouteArgsIfActive } from "./useRouteArgsIfActive"; -export { RoutingContext, TestRoutingContext } from "./providers"; +export { + RoutingContext, + TestRoutingContext, + useInRoutingContext, +} from "./providers"; diff --git a/src/routing/providers.tsx b/src/routing/providers.tsx index 67f75c4..19d7461 100644 --- a/src/routing/providers.tsx +++ b/src/routing/providers.tsx @@ -21,6 +21,15 @@ function useRoutingContext() { return context; } +/** + * @private + */ +export function useInRoutingContext(): boolean { + const context = useContext(RoutingContext); + + return context !== undefined; +} + export function useActiveRouteEvents() { try { const context = useRoutingContext(); diff --git a/src/xstateTree.spec.tsx b/src/xstateTree.spec.tsx index f7b0b88..06e2f04 100644 --- a/src/xstateTree.spec.tsx +++ b/src/xstateTree.spec.tsx @@ -1,5 +1,6 @@ import { render } from "@testing-library/react"; import { assign } from "@xstate/immer"; +import { createMemoryHistory } from "history"; import React from "react"; import { createMachine, interpret } from "xstate"; @@ -262,4 +263,81 @@ describe("xstate-tree", () => { expect(view2).toBe(getMultiSlotViewForChildren(interpreter2, "ignored")); }); }); + + describe("rendering a root inside of a root", () => { + it("throws an error during rendering if both are routing roots", async () => { + const machine = createMachine({ + id: "test", + initial: "idle", + states: { + idle: {}, + }, + }); + + const RootMachine = createXStateTreeMachine(machine, { + View() { + return

I am root

; + }, + }); + const Root = buildRootComponent(RootMachine, { + basePath: "/", + history: createMemoryHistory(), + routes: [], + }); + + const Root2Machine = createXStateTreeMachine(machine, { + View() { + return ; + }, + }); + const Root2 = buildRootComponent(Root2Machine, { + basePath: "/", + history: createMemoryHistory(), + routes: [], + }); + + try { + const { rerender } = render(); + rerender(); + } catch (e: any) { + expect(e.message).toMatchInlineSnapshot( + `"Routing root rendered inside routing context, this implies a bug"` + ); + return; + } + + throw new Error("Should have thrown"); + }); + + it("does not throw an error if either or one are a routing root", async () => { + const machine = createMachine({ + id: "test", + initial: "idle", + states: { + idle: {}, + }, + }); + + const RootMachine = createXStateTreeMachine(machine, { + View() { + return

I am root

; + }, + }); + const Root = buildRootComponent(RootMachine); + + const Root2Machine = createXStateTreeMachine(machine, { + View() { + return ; + }, + }); + const Root2 = buildRootComponent(Root2Machine, { + basePath: "/", + history: createMemoryHistory(), + routes: [], + }); + + const { rerender } = render(); + rerender(); + }); + }); }); diff --git a/src/xstateTree.tsx b/src/xstateTree.tsx index f9a3035..8647c05 100644 --- a/src/xstateTree.tsx +++ b/src/xstateTree.tsx @@ -23,6 +23,7 @@ import { RoutingContext, RoutingEvent, SharedMeta, + useInRoutingContext, } from "./routing"; import { useActiveRouteEvents } from "./routing/providers"; import { GetSlotNames, Slot } from "./slots"; @@ -353,6 +354,16 @@ export function buildRootComponent( const setActiveRouteEvents = (events: RoutingEvent[]) => { activeRouteEventsRef.current = events; }; + const insideRoutingContext = useInRoutingContext(); + if (insideRoutingContext && typeof routing !== "undefined") { + const m = + "Routing root rendered inside routing context, this implies a bug"; + if (process.env.NODE_ENV !== "production") { + throw new Error(m); + } + + console.error(m); + } useEffect(() => { function handler(event: GlobalEvents) { diff --git a/tsconfig.json b/tsconfig.json index e450ecc..90e47b3 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -19,7 +19,7 @@ "moduleResolution": "node", "esModuleInterop": true, "declarationMap": true, - "types": ["jest"], + "types": ["jest", "node"], "paths": { "@koordinates/xstate-tree": ["./src/index.ts"] },