Skip to content

Commit 3efa5d0

Browse files
brophdawg11brophdawg11
authored
Remove useSyncExternalStore in favor of useState (#10377)
1 parent 2821817 commit 3efa5d0

File tree

8 files changed

+38
-250
lines changed

8 files changed

+38
-250
lines changed

‎.changeset/remove-use-sync-external-store.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"react-router": patch
3+
"react-router-dom": patch
4+
---
5+
6+
Switched from `useSyncExternalStore` to `useState` for internal `@remix-run/router` router state syncing in `<RouterProvider>`. We found some [subtle bugs](https://codesandbox.io/s/use-sync-external-store-loop-9g7b81) where router state updates got propagated _before_ other normal `useState` updates, which could lead to footguns in `useEffect` calls.

‎package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,10 @@
108108
"none": "45.8 kB"
109109
},
110110
"packages/react-router/dist/react-router.production.min.js": {
111-
"none": "13.6 kB"
111+
"none": "12.9 kB"
112112
},
113113
"packages/react-router/dist/umd/react-router.production.min.js": {
114-
"none": "15.9 kB"
114+
"none": "15.3 kB"
115115
},
116116
"packages/react-router-dom/dist/react-router-dom.production.min.js": {
117117
"none": "12 kB"

‎packages/react-router-dom/__tests__/special-characters-test.tsx

Lines changed: 18 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -828,19 +828,8 @@ describe("special character tests", () => {
828828
});
829829

830830
describe("browser routers", () => {
831-
let testWindow: Window;
832-
833-
beforeEach(() => {
834-
// Need to use our own custom DOM in order to get a working history
835-
const dom = new JSDOM(`<!DOCTYPE html><html><body></body></html>`, {
836-
url: "https://remix.run/",
837-
});
838-
testWindow = dom.window as unknown as Window;
839-
testWindow.history.pushState({}, "", "/");
840-
});
841-
842831
it("encodes characters in BrowserRouter", () => {
843-
testWindow.history.replaceState(null, "", "/with space");
832+
let testWindow = getWindow("/with space");
844833

845834
let ctx = render(
846835
<BrowserRouter window={testWindow}>
@@ -857,7 +846,7 @@ describe("special character tests", () => {
857846
});
858847

859848
it("encodes characters in BrowserRouter (navigate)", () => {
860-
testWindow.history.replaceState(null, "", "/");
849+
let testWindow = getWindow("/");
861850

862851
function Start() {
863852
let navigate = useNavigate();
@@ -882,7 +871,7 @@ describe("special character tests", () => {
882871
});
883872

884873
it("encodes characters in createBrowserRouter", () => {
885-
testWindow.history.replaceState(null, "", "/with space");
874+
let testWindow = getWindow("/with space");
886875

887876
let router = createBrowserRouter(
888877
[{ path: "/with space", element: <ShowPath /> }],
@@ -897,7 +886,7 @@ describe("special character tests", () => {
897886
});
898887

899888
it("encodes characters in createBrowserRouter (navigate)", () => {
900-
testWindow.history.replaceState(null, "", "/with space");
889+
let testWindow = getWindow("/");
901890

902891
function Start() {
903892
let navigate = useNavigate();
@@ -923,19 +912,8 @@ describe("special character tests", () => {
923912
});
924913

925914
describe("hash routers", () => {
926-
let testWindow: Window;
927-
928-
beforeEach(() => {
929-
// Need to use our own custom DOM in order to get a working history
930-
const dom = new JSDOM(`<!DOCTYPE html><html><body></body></html>`, {
931-
url: "https://remix.run/",
932-
});
933-
testWindow = dom.window as unknown as Window;
934-
testWindow.history.pushState({}, "", "/");
935-
});
936-
937915
it("encodes characters in HashRouter", () => {
938-
testWindow.history.replaceState(null, "", "/#/with space");
916+
let testWindow = getWindow("/#/with space");
939917

940918
let ctx = render(
941919
<HashRouter window={testWindow}>
@@ -953,7 +931,7 @@ describe("special character tests", () => {
953931
});
954932

955933
it("encodes characters in HashRouter (navigate)", () => {
956-
testWindow.history.replaceState(null, "", "/");
934+
let testWindow = getWindow("/");
957935

958936
function Start() {
959937
let navigate = useNavigate();
@@ -979,7 +957,7 @@ describe("special character tests", () => {
979957
});
980958

981959
it("encodes characters in createHashRouter", () => {
982-
testWindow.history.replaceState(null, "", "/#/with space");
960+
let testWindow = getWindow("/#/with space");
983961

984962
let router = createHashRouter(
985963
[{ path: "/with space", element: <ShowPath /> }],
@@ -995,7 +973,7 @@ describe("special character tests", () => {
995973
});
996974

997975
it("encodes characters in createHashRouter (navigate)", () => {
998-
testWindow.history.replaceState(null, "", "/");
976+
let testWindow = getWindow("/");
999977

1000978
function Start() {
1001979
let navigate = useNavigate();
@@ -1022,3 +1000,13 @@ describe("special character tests", () => {
10221000
});
10231001
});
10241002
});
1003+
1004+
function getWindow(initialPath) {
1005+
// Need to use our own custom DOM in order to get a working history
1006+
const dom = new JSDOM(`<!DOCTYPE html><html><body></body></html>`, {
1007+
url: "https://remix.run/",
1008+
});
1009+
let testWindow = dom.window as unknown as Window;
1010+
testWindow.history.pushState({}, "", initialPath);
1011+
return testWindow;
1012+
}

‎packages/react-router/__tests__/data-memory-router-test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -887,7 +887,7 @@ describe("createMemoryRouter", () => {
887887

888888
spy.mockClear();
889889
fireEvent.click(screen.getByText("Link to Child"));
890-
await new Promise((r) => setTimeout(r, 0));
890+
await waitFor(() => screen.getByText("Child"));
891891

892892
expect(getHtml(container)).toMatchInlineSnapshot(`
893893
"<div>

‎packages/react-router/lib/components.tsx

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import type {
55
Location,
66
MemoryHistory,
77
Router as RemixRouter,
8-
RouterState,
98
To,
109
LazyRouteFunction,
1110
RelativeRoutingType,
@@ -19,7 +18,6 @@ import {
1918
stripBasename,
2019
UNSAFE_warning as warning,
2120
} from "@remix-run/router";
22-
import { useSyncExternalStore as useSyncExternalStoreShim } from "./use-sync-external-store-shim";
2321

2422
import type {
2523
DataRouteObject,
@@ -57,17 +55,17 @@ export function RouterProvider({
5755
fallbackElement,
5856
router,
5957
}: RouterProviderProps): React.ReactElement {
60-
let getState = React.useCallback(() => router.state, [router]);
61-
62-
// Sync router state to our component state to force re-renders
63-
let state: RouterState = useSyncExternalStoreShim(
64-
router.subscribe,
65-
getState,
66-
// We have to provide this so React@18 doesn't complain during hydration,
67-
// but we pass our serialized hydration data into the router so state here
68-
// is already synced with what the server saw
69-
getState
70-
);
58+
let [state, setState] = React.useState(router.state);
59+
60+
// Need to use a layout effect here so we are subscribed early enough to
61+
// pick up on any render-driven redirects/navigations (useEffect/<Navigate>)
62+
React.useLayoutEffect(() => {
63+
return router.subscribe((newState) => {
64+
if (newState !== state) {
65+
setState(newState);
66+
}
67+
});
68+
}, [router, state]);
7169

7270
let navigator = React.useMemo((): Navigator => {
7371
return {

‎packages/react-router/lib/use-sync-external-store-shim/index.ts

Lines changed: 0 additions & 32 deletions
This file was deleted.

‎packages/react-router/lib/use-sync-external-store-shim/useSyncExternalStoreShimClient.ts

Lines changed: 0 additions & 152 deletions
This file was deleted.

‎packages/react-router/lib/use-sync-external-store-shim/useSyncExternalStoreShimServer.ts

Lines changed: 0 additions & 20 deletions
This file was deleted.

0 commit comments

Comments
 (0)