From 54c132806659b9de1a05f2eef4d1675082cd87a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Best?= Date: Sun, 10 Dec 2023 15:30:02 +0100 Subject: [PATCH] fix: Push (#419) * test: Add push test * chore: Add push test on pages router * chore: Skip broken app router push test on latest * fix: Use pages router via internals Going through the history API only on shallow updates doesn't notify the pages router, which can handle shallow updates directly. Definitely a hack, but it allows also setting the asPath for dynamic routes. There is still an issue with basePath being applied twice, but that's an internal Next.js bug for another day. * chore: Fix workflow * chore: Restore test now that 14.0.4 has been released * chore: Use 14.0.4 as baseline * chore: Fix double basePath application Disabling the bloom filter causing invalid collisions between the pages and app routers. * chore: Restore hard fail on CI * test: Drop Next.js version injection (no longer needed) * doc: Add note about internals usage --- packages/e2e/cypress/e2e/push.cy.js | 90 ++++++++++++++++++ packages/e2e/next.config.mjs | 7 +- packages/e2e/src/app/app/push/client.tsx | 28 ++++++ packages/e2e/src/app/app/push/page.tsx | 18 ++++ packages/e2e/src/app/app/push/searchParams.ts | 5 + packages/e2e/src/pages/pages/push/index.tsx | 25 +++++ .../next-usequerystate/src/update-queue.ts | 95 ++++++++++++------- 7 files changed, 234 insertions(+), 34 deletions(-) create mode 100644 packages/e2e/cypress/e2e/push.cy.js create mode 100644 packages/e2e/src/app/app/push/client.tsx create mode 100644 packages/e2e/src/app/app/push/page.tsx create mode 100644 packages/e2e/src/app/app/push/searchParams.ts create mode 100644 packages/e2e/src/pages/pages/push/index.tsx diff --git a/packages/e2e/cypress/e2e/push.cy.js b/packages/e2e/cypress/e2e/push.cy.js new file mode 100644 index 00000000..37513dba --- /dev/null +++ b/packages/e2e/cypress/e2e/push.cy.js @@ -0,0 +1,90 @@ +/// + +describe('push', () => { + it('works in app router', () => { + cy.visit('/app/push') + cy.contains('#hydration-marker', 'hydrated').should('be.hidden') + cy.get('#server-side').should('have.text', '0') + cy.get('#server').should('have.text', '0') + cy.get('#client').should('have.text', '0') + + cy.get('button#server-incr').click() + cy.location('search').should('eq', '?server=1') + cy.get('#server-side').should('have.text', '1') + cy.get('#server').should('have.text', '1') + cy.get('#client').should('have.text', '0') + + cy.get('button#client-incr').click() + cy.location('search').should('eq', '?server=1&client=1') + cy.get('#server-side').should('have.text', '1') + cy.get('#server').should('have.text', '1') + cy.get('#client').should('have.text', '1') + + cy.go('back') + cy.location('search').should('eq', '?server=1') + cy.get('#server-side').should('have.text', '1') + cy.get('#server').should('have.text', '1') + cy.get('#client').should('have.text', '0') + + cy.go('back') + cy.location('search').should('be.empty') + cy.get('#server-side').should('have.text', '0') + cy.get('#server').should('have.text', '0') + cy.get('#client').should('have.text', '0') + + cy.go('forward') + cy.location('search').should('eq', '?server=1') + cy.get('#server-side').should('have.text', '1') + cy.get('#server').should('have.text', '1') + cy.get('#client').should('have.text', '0') + + cy.go('forward') + cy.location('search').should('eq', '?server=1&client=1') + cy.get('#server-side').should('have.text', '1') + cy.get('#server').should('have.text', '1') + cy.get('#client').should('have.text', '1') + }) + + it('works in pages router', () => { + cy.visit('/pages/push') + cy.get('#server-side').should('have.text', '0') + cy.get('#server').should('have.text', '0') + cy.get('#client').should('have.text', '0') + + cy.get('button#server-incr').click() + cy.location('search').should('eq', '?server=1') + cy.get('#server-side').should('have.text', '1') + cy.get('#server').should('have.text', '1') + cy.get('#client').should('have.text', '0') + + cy.get('button#client-incr').click() + cy.location('search').should('eq', '?server=1&client=1') + cy.get('#server-side').should('have.text', '1') + cy.get('#server').should('have.text', '1') + cy.get('#client').should('have.text', '1') + + cy.go('back') + cy.location('search').should('eq', '?server=1') + cy.get('#server-side').should('have.text', '1') + cy.get('#server').should('have.text', '1') + cy.get('#client').should('have.text', '0') + + cy.go('back') + cy.location('search').should('be.empty') + cy.get('#server-side').should('have.text', '0') + cy.get('#server').should('have.text', '0') + cy.get('#client').should('have.text', '0') + + cy.go('forward') + cy.location('search').should('eq', '?server=1') + cy.get('#server-side').should('have.text', '1') + cy.get('#server').should('have.text', '1') + cy.get('#client').should('have.text', '0') + + cy.go('forward') + cy.location('search').should('eq', '?server=1&client=1') + cy.get('#server-side').should('have.text', '1') + cy.get('#server').should('have.text', '1') + cy.get('#client').should('have.text', '1') + }) +}) diff --git a/packages/e2e/next.config.mjs b/packages/e2e/next.config.mjs index 329b926d..a02f8680 100644 --- a/packages/e2e/next.config.mjs +++ b/packages/e2e/next.config.mjs @@ -1,9 +1,12 @@ const experimental = process.env.WINDOW_HISTORY_SUPPORT === 'true' ? { - windowHistorySupport: true + windowHistorySupport: true, + clientRouterFilter: false + } + : { + clientRouterFilter: false } - : undefined const basePath = process.env.BASE_PATH === '/' ? undefined : process.env.BASE_PATH diff --git a/packages/e2e/src/app/app/push/client.tsx b/packages/e2e/src/app/app/push/client.tsx new file mode 100644 index 00000000..646594a5 --- /dev/null +++ b/packages/e2e/src/app/app/push/client.tsx @@ -0,0 +1,28 @@ +'use client' + +import { useQueryState } from 'next-usequerystate' +import { parser } from './searchParams' + +export function Client() { + const [client, setClient] = useQueryState('client', parser) + const [server, setServer] = useQueryState( + 'server', + parser.withOptions({ shallow: false }) + ) + return ( + <> +

+ Client: {client} +

+

+ Server: {server} +

+ + + + ) +} diff --git a/packages/e2e/src/app/app/push/page.tsx b/packages/e2e/src/app/app/push/page.tsx new file mode 100644 index 00000000..2a0f0120 --- /dev/null +++ b/packages/e2e/src/app/app/push/page.tsx @@ -0,0 +1,18 @@ +import { Client } from './client' +import { parser } from './searchParams' + +export default function Page({ + searchParams +}: { + searchParams: Record +}) { + const server = parser.parseServerSide(searchParams.server) + return ( + <> +

+ Server side: {server} +

+ + + ) +} diff --git a/packages/e2e/src/app/app/push/searchParams.ts b/packages/e2e/src/app/app/push/searchParams.ts new file mode 100644 index 00000000..f8528bf0 --- /dev/null +++ b/packages/e2e/src/app/app/push/searchParams.ts @@ -0,0 +1,5 @@ +import { parseAsInteger } from 'next-usequerystate' + +export const parser = parseAsInteger.withDefault(0).withOptions({ + history: 'push' +}) diff --git a/packages/e2e/src/pages/pages/push/index.tsx b/packages/e2e/src/pages/pages/push/index.tsx new file mode 100644 index 00000000..94197b1b --- /dev/null +++ b/packages/e2e/src/pages/pages/push/index.tsx @@ -0,0 +1,25 @@ +import { GetServerSideProps } from 'next' +import { Client } from '../../../app/app/push/client' +import { parser } from '../../../app/app/push/searchParams' + +export default function Page({ server }: { server: number }) { + return ( + <> +

+ Server side: {server} +

+ + + ) +} + +export const getServerSideProps = (async ctx => { + const server = parser.parseServerSide(ctx.query.server) + return { + props: { + server + } + } +}) satisfies GetServerSideProps<{ + server: number +}> diff --git a/packages/next-usequerystate/src/update-queue.ts b/packages/next-usequerystate/src/update-queue.ts index c60d6665..4670ed28 100644 --- a/packages/next-usequerystate/src/update-queue.ts +++ b/packages/next-usequerystate/src/update-queue.ts @@ -1,3 +1,4 @@ +import type { NextRouter } from 'next/router' import { debug } from './debug' import type { Options, Router } from './defs' import { error } from './errors' @@ -116,6 +117,18 @@ export function scheduleFlushToURL(router: Router) { return flushPromiseCache } +declare global { + interface Window { + next: { + router?: NextRouter & { + state: { + asPath: string + } + } + } + } +} + function flushUpdateQueue(router: Router): [URLSearchParams, null | unknown] { const search = new URLSearchParams(location.search) if (updateQueue.size === 0) { @@ -140,37 +153,56 @@ function flushUpdateQueue(router: Router): [URLSearchParams, null | unknown] { search.set(key, value) } } - const url = renderURL(search) - debug('[nuqs queue] Updating url: %s', url) - try { - // First, update the URL locally without triggering a network request, - // this allows keeping a reactive URL if the network is slow. - const updateMethod = - options.history === 'push' ? history.pushState : history.replaceState - updateMethod.call( - history, - history.state, - // Our own updates have a marker to prevent syncing - // when the URL changes (we've already sync'd them up - // via `emitter.emit(key, newValue)` above, without - // going through the parsers). - NOSYNC_MARKER, - url - ) - if (options.scroll) { - window.scrollTo(0, 0) - } - if (!options.shallow) { - compose(transitions, () => { - // Call the Next.js router to perform a network request - // and re-render server components. - router.replace(url, { - scroll: false, - // @ts-expect-error - pages router fix, but not exposed in navigation types - shallow: false - }) + // While the Next.js team doesn't recommend using internals like this, + // we need access to the pages router here to let it know about non-shallow + // updates, as going through the window.history API directly will make it + // miss pushed history updates. + // The router adapter imported from next/navigation also doesn't support + // passing an asPath, causing issues in dynamic routes in the pages router. + const nextRouter = window.next.router + const isPagesRouter = typeof nextRouter?.state?.asPath === 'string' + if (isPagesRouter) { + const url = renderURL(nextRouter.state.asPath.split('?')[0] ?? '', search) + debug('[nuqs queue (pages)] Updating url: %s', url) + const method = + options.history === 'push' ? nextRouter.push : nextRouter.replace + method.call(nextRouter, url, url, { + scroll: options.scroll, + shallow: options.shallow }) + } else { + // App router + const url = renderURL(location.href.split('?')[0] ?? '', search) + debug('[nuqs queue (app)] Updating url: %s', url) + // First, update the URL locally without triggering a network request, + // this allows keeping a reactive URL if the network is slow. + const updateMethod = + options.history === 'push' ? history.pushState : history.replaceState + updateMethod.call( + history, + history.state, + // Our own updates have a marker to prevent syncing + // when the URL changes (we've already sync'd them up + // via `emitter.emit(key, newValue)` above, without + // going through the parsers). + NOSYNC_MARKER, + url + ) + if (options.scroll) { + window.scrollTo(0, 0) + } + if (!options.shallow) { + compose(transitions, () => { + // Call the Next.js router to perform a network request + // and re-render server components. + router.replace(url, { + scroll: false, + // @ts-expect-error - pages router fix, but not exposed in navigation types + shallow: false + }) + }) + } } return [search, null] } catch (err) { @@ -181,11 +213,10 @@ function flushUpdateQueue(router: Router): [URLSearchParams, null | unknown] { } } -function renderURL(search: URLSearchParams) { +function renderURL(base: string, search: URLSearchParams) { const query = renderQueryString(search) - const href = location.href.split('?')[0] const hash = location.hash - return href + query + hash + return base + query + hash } export function compose(