From e911272c21241d2fb437e957d6bce9fce96e30de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Best?= Date: Tue, 14 Nov 2023 10:50:39 +0100 Subject: [PATCH] feat: Add support for experimental WHS (#394) * feat: Add support for experimental WHS Starting from next@14.0.3-canary.6, you can turn on the experimental `windowHistorySupport` flag to solve issues with prefetch links. There may be some optimisations to do since Next.js now also patches the history methods (and do so after nuqs), so it looks like there isn't a need for external sync anymore, but this first draft seems to work. Adding a few more CI cases to test for non-regression. * chore: Fix CI workflow * chore: Fix workflow name * chore: Only specify experimental when needed Helps with debugging * test: Fix compat for Next.js 14.0.1 and lower * chore: Shorter job names * chore: Fix basePath display * chore: Display next.js config * test: Test on raw canary but allow failures * test: Don't fail fast * chore: Allow all canaries to fail * chore: Add canary with basePath * chore: Actually test all with basePath * chore: De-flake repro-388 test It almost always passes in CI, but fails consistently locally. IT SHOULD FAIL!! * chore: Save Cypress artifacts on failure --- .github/workflows/ci-cd.yml | 32 ++++- .../workflows/test-against-nextjs-release.yml | 1 + packages/e2e/cypress.config.ts | 14 +- packages/e2e/cypress/e2e/internals.cy.js | 4 + packages/e2e/cypress/e2e/repro-388.cy.js | 2 + packages/e2e/next.config.mjs | 18 ++- packages/e2e/package.json | 2 +- packages/e2e/src/app/app/internals/page.tsx | 3 + .../next-usequerystate/src/update-queue.ts | 19 +-- .../next-usequerystate/src/useQueryState.ts | 10 ++ pnpm-lock.yaml | 128 +++++++++++++++++- turbo.json | 4 +- 12 files changed, 216 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index c1b42d03..17b3f208 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -10,19 +10,32 @@ env: jobs: ci: - name: Integration (Next.js ${{ matrix.next-version }}${{ matrix.base-path != '/' && ' with basePath' || ''}}) + # Watch out! When changing the job name, + # update the required checks in GitHub + # branch protection settings for `next`. + name: CI (${{ matrix.next-version }}${{ matrix.base-path && ' basePath' || ''}}${{ matrix.window-history-support && ' WHS' || ''}}) runs-on: ubuntu-latest + continue-on-error: ${{ matrix.next-version == 'canary' }} strategy: + fail-fast: false matrix: - base-path: ['/'] + # Watch out! When changing the compat grid, + # update the required checks in GitHub + # branch protection settings for `next`. + base-path: [false, '/base'] + window-history-support: [false] next-version: - '13.4' - '13.5' - '14.0.1' - # - latest + - canary include: - - next-version: '14.0.1' + - next-version: canary + window-history-support: true + - next-version: canary + window-history-support: true base-path: '/base' + steps: - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 - uses: pnpm/action-setup@d882d12c64e032187b2edb46d3a0d003b7a43598 @@ -39,15 +52,22 @@ jobs: - name: Run integration tests run: pnpm run test env: - BASE_PATH: ${{ matrix.base-path }} + BASE_PATH: ${{ matrix.base-path && matrix.base-path || '/' }} + WINDOW_HISTORY_SUPPORT: ${{ matrix.window-history-support }} TURBO_TOKEN: ${{ secrets.TURBO_TOKEN }} TURBO_TEAM: ${{ secrets.TURBO_TEAM }} + - name: Save Cypress artifacts + uses: actions/upload-artifact@a8a3f3ad30e3422c9c7b888a15615d19a852ae32 + if: failure() + with: + path: packages/e2e/cypress/screenshots + name: ci-${{ matrix.next-version }}${{ matrix.base-path && '-basePath' || ''}}${{ matrix.window-history-support && '-whs' || ''}} - uses: 47ng/actions-slack-notify@main name: Notify on Slack if: always() with: status: ${{ job.status }} - jobName: next@${{ matrix.next-version }} + jobName: next@${{ matrix.next-version }}${{ matrix.base-path && ' basePath' || ''}}${{ matrix.window-history-support && ' WHS' || ''}} env: SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} diff --git a/.github/workflows/test-against-nextjs-release.yml b/.github/workflows/test-against-nextjs-release.yml index 67c90b93..79bc4494 100644 --- a/.github/workflows/test-against-nextjs-release.yml +++ b/.github/workflows/test-against-nextjs-release.yml @@ -32,6 +32,7 @@ jobs: - name: Run integration tests run: pnpm run test env: + WINDOW_HISTORY_SUPPORT: 'true' TURBO_TOKEN: ${{ secrets.TURBO_TOKEN }} TURBO_TEAM: ${{ secrets.TURBO_TEAM }} - uses: 47ng/actions-slack-notify@main diff --git a/packages/e2e/cypress.config.ts b/packages/e2e/cypress.config.ts index 6be022c0..dfdfa25c 100644 --- a/packages/e2e/cypress.config.ts +++ b/packages/e2e/cypress.config.ts @@ -1,8 +1,19 @@ import { defineConfig } from 'cypress' +import fs from 'node:fs' + +const pkgPath = new URL('./package.json', import.meta.url) +const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf-8')) const basePath = process.env.BASE_PATH === '/' ? '' : process.env.BASE_PATH ?? '' +const windowHistorySupport = + pkg.dependencies.next >= '14.0.3-canary.6' + ? process.env.WINDOW_HISTORY_SUPPORT === 'true' + ? 'true' + : 'false' + : 'undefined' + export default defineConfig({ e2e: { baseUrl: `http://localhost:3001${basePath}`, @@ -12,7 +23,8 @@ export default defineConfig({ testIsolation: true, retries: 5, env: { - basePath + basePath, + windowHistorySupport } } }) diff --git a/packages/e2e/cypress/e2e/internals.cy.js b/packages/e2e/cypress/e2e/internals.cy.js index 85c8839f..6768f3a4 100644 --- a/packages/e2e/cypress/e2e/internals.cy.js +++ b/packages/e2e/cypress/e2e/internals.cy.js @@ -6,6 +6,10 @@ describe('internals', () => { cy.contains('#hydration-marker', 'hydrated').should('be.hidden') cy.get('#__N').should('have.text', 'undefined') cy.get('#__NA').should('have.text', 'true') + cy.get('#windowHistorySupport').should( + 'have.text', + Cypress.env('windowHistorySupport') + ) }) it('works in pages router', () => { diff --git a/packages/e2e/cypress/e2e/repro-388.cy.js b/packages/e2e/cypress/e2e/repro-388.cy.js index 57a25540..ddc3be63 100644 --- a/packages/e2e/cypress/e2e/repro-388.cy.js +++ b/packages/e2e/cypress/e2e/repro-388.cy.js @@ -12,6 +12,7 @@ it('Reproduction for issue #388', () => { cy.get('#counter').should('have.text', 'Counter: 1') // Hover the "Hover me" link cy.get('#hover-me').trigger('mouseover') + cy.wait(100) // The URL should have a ?counter=1 query string cy.location('search').should('eq', '?counter=1') // The counter should be rendered as 1 on the page @@ -26,6 +27,7 @@ it('Reproduction for issue #388', () => { cy.get('#counter').should('have.text', 'Counter: 1') // Mount the other link cy.get('#toggle').click() + cy.wait(100) // The URL should have a ?counter=1 query string cy.location('search').should('eq', '?counter=1') // The counter should be rendered as 1 on the page diff --git a/packages/e2e/next.config.mjs b/packages/e2e/next.config.mjs index 9e0699e2..329b926d 100644 --- a/packages/e2e/next.config.mjs +++ b/packages/e2e/next.config.mjs @@ -1,6 +1,22 @@ +const experimental = + process.env.WINDOW_HISTORY_SUPPORT === 'true' + ? { + windowHistorySupport: true + } + : undefined + +const basePath = + process.env.BASE_PATH === '/' ? undefined : process.env.BASE_PATH + /** @type {import('next').NextConfig } */ const config = { - basePath: process.env.BASE_PATH === '/' ? undefined : process.env.BASE_PATH + basePath, + experimental } +console.info(`Next.js config: + basePath: ${basePath} + windowHistorySupport: ${experimental?.windowHistorySupport ?? false} +`) + export default config diff --git a/packages/e2e/package.json b/packages/e2e/package.json index 4d951ffa..30f20ba4 100644 --- a/packages/e2e/package.json +++ b/packages/e2e/package.json @@ -20,7 +20,7 @@ "cypress:run": "cypress run --headless" }, "dependencies": { - "next": "14.0.1", + "next": "14.0.3-canary.6", "next-usequerystate": "workspace:*", "react": "^18.2.0", "react-dom": "^18.2.0" diff --git a/packages/e2e/src/app/app/internals/page.tsx b/packages/e2e/src/app/app/internals/page.tsx index b41ece1c..80afa40e 100644 --- a/packages/e2e/src/app/app/internals/page.tsx +++ b/packages/e2e/src/app/app/internals/page.tsx @@ -11,6 +11,9 @@ export default function Page() { <>

{String(history.state.__N)}

{String(history.state.__NA)}

+

+ {String(process.env.__NEXT_WINDOW_HISTORY_SUPPORT)} +

) } diff --git a/packages/next-usequerystate/src/update-queue.ts b/packages/next-usequerystate/src/update-queue.ts index a2bac014..1716ff2e 100644 --- a/packages/next-usequerystate/src/update-queue.ts +++ b/packages/next-usequerystate/src/update-queue.ts @@ -172,17 +172,20 @@ function flushUpdateQueue(router: Router): [URLSearchParams, null | unknown] { function renderURL(search: URLSearchParams) { const query = renderQueryString(search) - // @ts-expect-error - Not exposed in types - const basePath = window?.next?.router?.basePath ?? '' - const path = location.pathname.slice(basePath.length) + const href = location.href.split('?')[0] const hash = location.hash if (history.state.__N === true) { - // Pages router: always use a full path to handle dynamic routes - return query ? `${path}?${query}${hash}` : `${path}${hash}` + // Pages router: always use a full path to handle dynamic routes, + // but strip the basePath to avoid recursively applying it. + // @ts-expect-error - Not exposed in types + const basePath = window?.next?.router?.basePath ?? '' + const path = location.pathname.slice(basePath.length) + const base = process.env.__NEXT_WINDOW_HISTORY_SUPPORT ? href : path + return query ? `${base}?${query}${hash}` : `${base}${hash}` } else { // App router - // If the querystring is empty, add the pathname to clear it out, - // otherwise using a relative URL works just fine. - return query ? `?${query}${hash}` : `${path}${hash}` + // From 12.0.3-canary.6, with the experimental windowHistorySupport flag, + // a valid URL is required, so prepend the href (but without the query string) + return query ? `${href}?${query}${hash}` : `${href}${hash}` } } diff --git a/packages/next-usequerystate/src/useQueryState.ts b/packages/next-usequerystate/src/useQueryState.ts index 7b627f5f..357f36c3 100644 --- a/packages/next-usequerystate/src/useQueryState.ts +++ b/packages/next-usequerystate/src/useQueryState.ts @@ -237,6 +237,16 @@ export function useQueryState( initialSearchParams?.get(key) ?? null ) + if (process.env.__NEXT_WINDOW_HISTORY_SUPPORT) { + React.useEffect(() => { + const value = initialSearchParams.get(key) ?? null + const state = value === null ? null : parse(value) + debug(`[nuqs \`%s\`] syncFromUseSearchParams %O`, key, state) + stateRef.current = state + setInternalState(state) + }, [initialSearchParams?.get(key), key]) + } + // Sync all hooks together & with external URL changes React.useInsertionEffect(() => { function updateInternalState(state: T | null) { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4a0765cc..11907e74 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -33,8 +33,8 @@ importers: packages/e2e: dependencies: next: - specifier: 14.0.1 - version: 14.0.1(react-dom@18.2.0)(react@18.2.0) + specifier: 14.0.3-canary.6 + version: 14.0.3-canary.6(react-dom@18.2.0)(react@18.2.0) next-usequerystate: specifier: workspace:* version: link:../next-usequerystate @@ -611,6 +611,10 @@ packages: /@next/env@14.0.1: resolution: {integrity: sha512-Ms8ZswqY65/YfcjrlcIwMPD7Rg/dVjdLapMcSHG26W6O67EJDF435ShW4H4LXi1xKO1oRc97tLXUpx8jpLe86A==} + /@next/env@14.0.3-canary.6: + resolution: {integrity: sha512-tTTIcpA0QaZ0poNcsR2sHeuwIybn/3kyUIaig6RKh63UU7cHOaGZzwj/9+qY+Krx00QLDdbKxGb+d0Uu6Iqo7g==} + dev: false + /@next/swc-darwin-arm64@14.0.1: resolution: {integrity: sha512-JyxnGCS4qT67hdOKQ0CkgFTp+PXub5W1wsGvIq98TNbF3YEIN7iDekYhYsZzc8Ov0pWEsghQt+tANdidITCLaw==} engines: {node: '>= 10'} @@ -619,6 +623,15 @@ packages: requiresBuild: true optional: true + /@next/swc-darwin-arm64@14.0.3-canary.6: + resolution: {integrity: sha512-Ja+2wV+X4l3REv4HiIskFHTj7SQUxZf2BnQjwwKzVpkdjDMCZCkUnACFlb4fFVP4cNl0d7w8qhosFnecrasw4g==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [darwin] + requiresBuild: true + dev: false + optional: true + /@next/swc-darwin-x64@14.0.1: resolution: {integrity: sha512-625Z7bb5AyIzswF9hvfZWa+HTwFZw+Jn3lOBNZB87lUS0iuCYDHqk3ujuHCkiyPtSC0xFBtYDLcrZ11mF/ap3w==} engines: {node: '>= 10'} @@ -627,6 +640,15 @@ packages: requiresBuild: true optional: true + /@next/swc-darwin-x64@14.0.3-canary.6: + resolution: {integrity: sha512-bFLXAndkOIrKwD32Yb+3AubUvZfK1Z4XiUQxUDiJnBCWuYruYZ9fGuJ8ucsodRr71R1rthnno6M6M8hafWpcvA==} + engines: {node: '>= 10'} + cpu: [x64] + os: [darwin] + requiresBuild: true + dev: false + optional: true + /@next/swc-linux-arm64-gnu@14.0.1: resolution: {integrity: sha512-iVpn3KG3DprFXzVHM09kvb//4CNNXBQ9NB/pTm8LO+vnnnaObnzFdS5KM+w1okwa32xH0g8EvZIhoB3fI3mS1g==} engines: {node: '>= 10'} @@ -635,6 +657,15 @@ packages: requiresBuild: true optional: true + /@next/swc-linux-arm64-gnu@14.0.3-canary.6: + resolution: {integrity: sha512-8ER9mlQL4jz1HmKK//Yv/MSG/EVk45NcCshg1SWE0TzcVr87N5dyBIPuw22GIVM2PSqo0Kf8vaNVn9Su4ErcJw==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [linux] + requiresBuild: true + dev: false + optional: true + /@next/swc-linux-arm64-musl@14.0.1: resolution: {integrity: sha512-mVsGyMxTLWZXyD5sen6kGOTYVOO67lZjLApIj/JsTEEohDDt1im2nkspzfV5MvhfS7diDw6Rp/xvAQaWZTv1Ww==} engines: {node: '>= 10'} @@ -643,6 +674,15 @@ packages: requiresBuild: true optional: true + /@next/swc-linux-arm64-musl@14.0.3-canary.6: + resolution: {integrity: sha512-tepB7suiUPFmjBvG25sMrfD59Z1Rz7GOkY1/C7iOHCxLicJ7Hk0ZNTFnHfB9R+KQewhSEhYXIPux89nnWz/qDw==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [linux] + requiresBuild: true + dev: false + optional: true + /@next/swc-linux-x64-gnu@14.0.1: resolution: {integrity: sha512-wMqf90uDWN001NqCM/auRl3+qVVeKfjJdT9XW+RMIOf+rhUzadmYJu++tp2y+hUbb6GTRhT+VjQzcgg/QTD9NQ==} engines: {node: '>= 10'} @@ -651,6 +691,15 @@ packages: requiresBuild: true optional: true + /@next/swc-linux-x64-gnu@14.0.3-canary.6: + resolution: {integrity: sha512-MH8J2E//P2DQdIr8ed7kpnzsAiXOH2+fRKcpEptXf/b6pCd3hhQU3ZgY+bhbrQIyfJhFTvJ5gaewErr3WVi68w==} + engines: {node: '>= 10'} + cpu: [x64] + os: [linux] + requiresBuild: true + dev: false + optional: true + /@next/swc-linux-x64-musl@14.0.1: resolution: {integrity: sha512-ol1X1e24w4j4QwdeNjfX0f+Nza25n+ymY0T2frTyalVczUmzkVD7QGgPTZMHfR1aLrO69hBs0G3QBYaj22J5GQ==} engines: {node: '>= 10'} @@ -659,6 +708,15 @@ packages: requiresBuild: true optional: true + /@next/swc-linux-x64-musl@14.0.3-canary.6: + resolution: {integrity: sha512-D3ETz2gF1Gh8l/ok7PwR51UDbnHti+HZxKMj8RD/XI89vzGjCHsoqG6/jZvU3EkNFxLy+7s8H2RxZ+i7FRqCFA==} + engines: {node: '>= 10'} + cpu: [x64] + os: [linux] + requiresBuild: true + dev: false + optional: true + /@next/swc-win32-arm64-msvc@14.0.1: resolution: {integrity: sha512-WEmTEeWs6yRUEnUlahTgvZteh5RJc4sEjCQIodJlZZ5/VJwVP8p2L7l6VhzQhT4h7KvLx/Ed4UViBdne6zpIsw==} engines: {node: '>= 10'} @@ -667,6 +725,15 @@ packages: requiresBuild: true optional: true + /@next/swc-win32-arm64-msvc@14.0.3-canary.6: + resolution: {integrity: sha512-0VeorcCq2K1JO7zP0TpSvusf/lLrcREAYq0U/pb/yRTnrMzc8croNuiiMQwD3HJSeXYeib14wn/i8LZApE2fwQ==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [win32] + requiresBuild: true + dev: false + optional: true + /@next/swc-win32-ia32-msvc@14.0.1: resolution: {integrity: sha512-oFpHphN4ygAgZUKjzga7SoH2VGbEJXZa/KL8bHCAwCjDWle6R1SpiGOdUdA8EJ9YsG1TYWpzY6FTbUA+iAJeww==} engines: {node: '>= 10'} @@ -675,6 +742,15 @@ packages: requiresBuild: true optional: true + /@next/swc-win32-ia32-msvc@14.0.3-canary.6: + resolution: {integrity: sha512-8o0c5d2IQQyMt/PsWdsYHvRFLVI2FxbgoBGDsqEYwKtXGgzHvLF9yh4cvYoCRDfmqfQ/ziH3YUO5NXL2FihOIA==} + engines: {node: '>= 10'} + cpu: [ia32] + os: [win32] + requiresBuild: true + dev: false + optional: true + /@next/swc-win32-x64-msvc@14.0.1: resolution: {integrity: sha512-FFp3nOJ/5qSpeWT0BZQ+YE1pSMk4IMpkME/1DwKBwhg4mJLB9L+6EXuJi4JEwaJdl5iN+UUlmUD3IsR1kx5fAg==} engines: {node: '>= 10'} @@ -683,6 +759,15 @@ packages: requiresBuild: true optional: true + /@next/swc-win32-x64-msvc@14.0.3-canary.6: + resolution: {integrity: sha512-cUyzas15zeQSH418+LtcMCDqu+CN9tJ7U57xAdkWmU6T8WMAy6E1UqKITgXz/fW8l/rz+g4AaHQQLBgQZ94Rdw==} + engines: {node: '>= 10'} + cpu: [x64] + os: [win32] + requiresBuild: true + dev: false + optional: true + /@nodelib/fs.scandir@2.1.5: resolution: {integrity: sha512-vq24Bq3ym5HEQm2NKCr3yXDwjc7vTsEThRDnkp2DK9p1uqLR+DHurm/NOTo0KG7HYHU7eppKZj3MyqYuMBf62g==} engines: {node: '>= 8'} @@ -3706,6 +3791,45 @@ packages: - '@babel/core' - babel-plugin-macros + /next@14.0.3-canary.6(react-dom@18.2.0)(react@18.2.0): + resolution: {integrity: sha512-Wn5ebhKQwbDG/RByKalJ8MufJhXvEyw7BZnJeDy2vFIU3wD0tcgvsi6/BPZemKvKPqJdcmSBRRrBBYQ/CcSrgg==} + engines: {node: '>=18.17.0'} + hasBin: true + peerDependencies: + '@opentelemetry/api': ^1.1.0 + react: ^18.2.0 + react-dom: ^18.2.0 + sass: ^1.3.0 + peerDependenciesMeta: + '@opentelemetry/api': + optional: true + sass: + optional: true + dependencies: + '@next/env': 14.0.3-canary.6 + '@swc/helpers': 0.5.2 + busboy: 1.6.0 + caniuse-lite: 1.0.30001561 + postcss: 8.4.31 + react: 18.2.0 + react-dom: 18.2.0(react@18.2.0) + styled-jsx: 5.1.1(react@18.2.0) + watchpack: 2.4.0 + optionalDependencies: + '@next/swc-darwin-arm64': 14.0.3-canary.6 + '@next/swc-darwin-x64': 14.0.3-canary.6 + '@next/swc-linux-arm64-gnu': 14.0.3-canary.6 + '@next/swc-linux-arm64-musl': 14.0.3-canary.6 + '@next/swc-linux-x64-gnu': 14.0.3-canary.6 + '@next/swc-linux-x64-musl': 14.0.3-canary.6 + '@next/swc-win32-arm64-msvc': 14.0.3-canary.6 + '@next/swc-win32-ia32-msvc': 14.0.3-canary.6 + '@next/swc-win32-x64-msvc': 14.0.3-canary.6 + transitivePeerDependencies: + - '@babel/core' + - babel-plugin-macros + dev: false + /nice-try@1.0.5: resolution: {integrity: sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ==} dev: true diff --git a/turbo.json b/turbo.json index e7be41b7..e6b08b1d 100644 --- a/turbo.json +++ b/turbo.json @@ -14,7 +14,7 @@ "e2e#build": { "outputs": [".next/**", "!.next/cache/**"], "dependsOn": ["^build"], - "env": ["BASE_PATH"] + "env": ["BASE_PATH", "WINDOW_HISTORY_SUPPORT"] }, "playground#build": { "outputs": [".next/**", "!.next/cache/**"], @@ -26,7 +26,7 @@ }, "e2e#test": { "dependsOn": ["build"], - "outputs": ["cypress/**"] + "outputs": ["cypress/**", "cypress.config.ts"] } } }