Skip to content

Commit

Permalink
fix(routing): notify about double routing roots
Browse files Browse the repository at this point in the history
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
  • Loading branch information
UberMouse committed Aug 9, 2023
1 parent 4640a52 commit 00eec3d
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 11 deletions.
13 changes: 7 additions & 6 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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/**/*'",
Expand Down
2 changes: 1 addition & 1 deletion src/routing/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export function Link<TRoute extends AnyRoute>({
// and everything that consumes params/query already checks for undefined
const { params, query, meta, ...props } = rest;

let timeout: number | undefined;
let timeout: ReturnType<typeof setTimeout> | undefined;
const href = useHref(to, params, query);
const onMouseDown: React.MouseEventHandler<HTMLAnchorElement> | undefined =
preloadOnInteraction
Expand Down
6 changes: 5 additions & 1 deletion src/routing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,8 @@ export {
export { useIsRouteActive } from "./useIsRouteActive";
export { useRouteArgsIfActive } from "./useRouteArgsIfActive";

export { RoutingContext, TestRoutingContext } from "./providers";
export {
RoutingContext,
TestRoutingContext,
useInRoutingContext,
} from "./providers";
9 changes: 9 additions & 0 deletions src/routing/providers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
78 changes: 78 additions & 0 deletions src/xstateTree.spec.tsx
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -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 <p>I am root</p>;
},
});
const Root = buildRootComponent(RootMachine, {
basePath: "/",
history: createMemoryHistory(),
routes: [],
});

const Root2Machine = createXStateTreeMachine(machine, {
View() {
return <Root />;
},
});
const Root2 = buildRootComponent(Root2Machine, {
basePath: "/",
history: createMemoryHistory(),
routes: [],
});

try {
const { rerender } = render(<Root2 />);
rerender(<Root2 />);
} 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 <p>I am root</p>;
},
});
const Root = buildRootComponent(RootMachine);

const Root2Machine = createXStateTreeMachine(machine, {
View() {
return <Root />;
},
});
const Root2 = buildRootComponent(Root2Machine, {
basePath: "/",
history: createMemoryHistory(),
routes: [],
});

const { rerender } = render(<Root2 />);
rerender(<Root2 />);
});
});
});
11 changes: 11 additions & 0 deletions src/xstateTree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
RoutingContext,
RoutingEvent,
SharedMeta,
useInRoutingContext,
} from "./routing";
import { useActiveRouteEvents } from "./routing/providers";
import { GetSlotNames, Slot } from "./slots";
Expand Down Expand Up @@ -353,6 +354,16 @@ export function buildRootComponent(
const setActiveRouteEvents = (events: RoutingEvent<any>[]) => {
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) {
Expand Down
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"moduleResolution": "node",
"esModuleInterop": true,
"declarationMap": true,
"types": ["jest"],
"types": ["jest", "node"],
"paths": {
"@koordinates/xstate-tree": ["./src/index.ts"]
},
Expand Down

0 comments on commit 00eec3d

Please sign in to comment.