-
Notifications
You must be signed in to change notification settings - Fork 326
fix(router): share pages useRouter state through context #1271
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import { isExternalUrl, isHashOnlyChange } from "../packages/vinext/src/shims/ro | |
| import { isValidModulePath } from "../packages/vinext/src/client/validate-module-path.js"; | ||
| import vinext from "../packages/vinext/src/index.js"; | ||
| import type { Plugin } from "vite-plus"; | ||
| import type { NextRouter } from "../packages/vinext/src/shims/router.js"; | ||
| import type { | ||
| CacheHandler, | ||
| CacheHandlerValue, | ||
|
|
@@ -1518,6 +1519,31 @@ describe("window.next debug global", () => { | |
| describe("next/router withRouter HOC", () => { | ||
| let previousWindow: unknown; | ||
|
|
||
| function createTestRouter(overrides: Partial<NextRouter> = {}): NextRouter { | ||
| const router: NextRouter = { | ||
| pathname: "/provided", | ||
| route: "/provided", | ||
| query: {}, | ||
| asPath: "/provided", | ||
| basePath: "", | ||
| isReady: true, | ||
| isPreview: false, | ||
| isFallback: false, | ||
| push: vi.fn(async () => true), | ||
| replace: vi.fn(async () => true), | ||
| back: vi.fn(), | ||
| reload: vi.fn(), | ||
| prefetch: vi.fn(async () => {}), | ||
| beforePopState: vi.fn(), | ||
| events: { | ||
| on: vi.fn(), | ||
| off: vi.fn(), | ||
| emit: vi.fn(), | ||
| }, | ||
| }; | ||
| return { ...router, ...overrides }; | ||
| } | ||
|
|
||
| beforeEach(() => { | ||
| previousWindow = (globalThis as any).window; | ||
| (globalThis as any).window = { | ||
|
|
@@ -1538,6 +1564,107 @@ describe("next/router withRouter HOC", () => { | |
| expect(typeof withRouter).toBe("function"); | ||
| }); | ||
|
|
||
| it("next/router useRouter reads the mounted RouterContext value", async () => { | ||
| const React = await import("react"); | ||
| const { renderToStaticMarkup } = await import("react-dom/server"); | ||
| const { useRouter } = await import("../packages/vinext/src/shims/router.js"); | ||
| const { RouterContext } = | ||
| await import("../packages/vinext/src/shims/internal/router-context.js"); | ||
|
|
||
| const providedRouter = createTestRouter({ pathname: "/from-context" }); | ||
| let captured: NextRouter | null = null; | ||
|
|
||
| function Probe() { | ||
| captured = useRouter(); | ||
| return React.createElement("span", null, "ok"); | ||
| } | ||
|
|
||
| renderToStaticMarkup( | ||
| React.createElement( | ||
| RouterContext.Provider, | ||
| { value: providedRouter }, | ||
| React.createElement(Probe), | ||
| ), | ||
| ); | ||
|
|
||
| expect(captured).toBe(providedRouter); | ||
| }); | ||
|
|
||
| it("next/router useRouter throws when the Pages Router context is not mounted", async () => { | ||
| const React = await import("react"); | ||
| const { renderToStaticMarkup } = await import("react-dom/server"); | ||
| const { useRouter } = await import("../packages/vinext/src/shims/router.js"); | ||
|
|
||
| function Probe() { | ||
| useRouter(); | ||
| return React.createElement("span", null, "ok"); | ||
| } | ||
|
|
||
| expect(() => renderToStaticMarkup(React.createElement(Probe))).toThrow( | ||
| "NextRouter was not mounted", | ||
| ); | ||
| }); | ||
|
|
||
| it("next/router useRouter does not subscribe once per hook call", async () => { | ||
| const previousWindowForMock = (globalThis as any).window; | ||
| const addEventListener = vi.fn(); | ||
| const providedRouter = createTestRouter(); | ||
|
|
||
| (globalThis as any).window = { | ||
| location: { pathname: "/", search: "", hash: "", href: "http://localhost/" }, | ||
| history: { state: null, pushState() {}, replaceState() {} }, | ||
| addEventListener, | ||
| removeEventListener: vi.fn(), | ||
| __NEXT_DATA__: { page: "/", query: {}, isFallback: false }, | ||
| }; | ||
|
|
||
| vi.resetModules(); | ||
| vi.doMock("react", () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: this React mock covers exactly the hooks |
||
| const react = { | ||
| createContext(defaultValue: unknown) { | ||
| return { Provider: "Provider", Consumer: "Consumer", defaultValue }; | ||
| }, | ||
| createElement(type: unknown, props: unknown, ...children: unknown[]) { | ||
| return { type, props, children }; | ||
| }, | ||
| useContext() { | ||
| return providedRouter; | ||
| }, | ||
| useState(initialValue: unknown) { | ||
| return [typeof initialValue === "function" ? initialValue() : initialValue, vi.fn()]; | ||
| }, | ||
| useEffect(effect: () => void | (() => void)) { | ||
| effect(); | ||
| }, | ||
| useMemo(factory: () => unknown) { | ||
| return factory(); | ||
| }, | ||
| }; | ||
| return { ...react, default: react }; | ||
| }); | ||
|
|
||
| try { | ||
| const { useRouter } = await import("../packages/vinext/src/shims/router.js"); | ||
|
|
||
| expect(useRouter()).toBe(providedRouter); | ||
| expect(useRouter()).toBe(providedRouter); | ||
| expect(useRouter()).toBe(providedRouter); | ||
|
|
||
| const navigateListenerCalls = addEventListener.mock.calls.filter( | ||
| (call) => call[0] === "vinext:navigate", | ||
| ); | ||
| expect(navigateListenerCalls).toHaveLength(0); | ||
| } finally { | ||
| vi.doUnmock("react"); | ||
| if (previousWindowForMock === undefined) { | ||
| delete (globalThis as any).window; | ||
| } else { | ||
| (globalThis as any).window = previousWindowForMock; | ||
| } | ||
| vi.resetModules(); | ||
| } | ||
| }); | ||
|
|
||
| it("withRouter wraps a component and forwards static props", async () => { | ||
| const { withRouter } = await import("../packages/vinext/src/shims/router.js"); | ||
| const React = await import("react"); | ||
|
|
@@ -1554,7 +1681,8 @@ describe("next/router withRouter HOC", () => { | |
| }); | ||
|
|
||
| it("withRouter injects a router prop into the wrapped component", async () => { | ||
| const { withRouter } = await import("../packages/vinext/src/shims/router.js"); | ||
| const { withRouter, wrapWithRouterContext } = | ||
| await import("../packages/vinext/src/shims/router.js"); | ||
| const React = await import("react"); | ||
| const { renderToStaticMarkup } = await import("react-dom/server"); | ||
|
|
||
|
|
@@ -1567,7 +1695,9 @@ describe("next/router withRouter HOC", () => { | |
| }; | ||
|
|
||
| const Wrapped = withRouter(Inner); | ||
| const html = renderToStaticMarkup(React.createElement(Wrapped as any, { label: "hi" })); | ||
| const html = renderToStaticMarkup( | ||
| wrapWithRouterContext(React.createElement(Wrapped, { label: "hi" })), | ||
| ); | ||
| expect(html).toBe("<span>ok</span>"); | ||
| expect(receivedLabel).toBe("hi"); | ||
| // router must be the NextRouter shape (push/replace/back/...). | ||
|
|
@@ -1587,7 +1717,8 @@ describe("next/router withRouter HOC", () => { | |
| // so a user-passed `router` prop overrides the HOC-injected one. If the | ||
| // spread order is ever inverted in the shim, this test fails. | ||
| it("user-passed router prop overrides the HOC-injected router (Next.js spread order)", async () => { | ||
| const { withRouter } = await import("../packages/vinext/src/shims/router.js"); | ||
| const { withRouter, wrapWithRouterContext } = | ||
| await import("../packages/vinext/src/shims/router.js"); | ||
| const React = await import("react"); | ||
| const { renderToStaticMarkup } = await import("react-dom/server"); | ||
|
|
||
|
|
@@ -1599,7 +1730,10 @@ describe("next/router withRouter HOC", () => { | |
|
|
||
| const Wrapped = withRouter(Inner); | ||
| const userRouter = { sentinel: "user-provided" }; | ||
| renderToStaticMarkup(React.createElement(Wrapped as any, { router: userRouter })); | ||
| const WrappedWithOverride = Wrapped as React.ComponentType<{ router: unknown }>; | ||
| renderToStaticMarkup( | ||
| wrapWithRouterContext(React.createElement(WrappedWithOverride, { router: userRouter })), | ||
| ); | ||
| // Last spread wins: the user-passed router survives. | ||
| expect(receivedRouter).toBe(userRouter); | ||
| }); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: the
useEffecton line 670 unconditionally referenceswindow, which would throw if this component somehow rendered on the server outside React's renderer (e.g., if someone called the effect directly in a test). In practice this is fine because React skipsuseEffectduring SSR, so the callback never runs. But atypeof window !== "undefined"guard inside the effect would make it defensive.Not blocking —
useEffecthas been a server no-op since React 16 and that contract is stable.