Skip to content

Commit

Permalink
feat: Add support for experimental WHS (#394)
Browse files Browse the repository at this point in the history
* feat: Add support for experimental WHS

Starting from [email protected], 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
  • Loading branch information
franky47 authored Nov 14, 2023
1 parent 1dc6288 commit e911272
Show file tree
Hide file tree
Showing 12 changed files with 216 additions and 21 deletions.
32 changes: 26 additions & 6 deletions .github/workflows/ci-cd.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 }}

Expand Down
1 change: 1 addition & 0 deletions .github/workflows/test-against-nextjs-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 13 additions & 1 deletion packages/e2e/cypress.config.ts
Original file line number Diff line number Diff line change
@@ -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}`,
Expand All @@ -12,7 +23,8 @@ export default defineConfig({
testIsolation: true,
retries: 5,
env: {
basePath
basePath,
windowHistorySupport
}
}
})
4 changes: 4 additions & 0 deletions packages/e2e/cypress/e2e/internals.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/e2e/cypress/e2e/repro-388.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
18 changes: 17 additions & 1 deletion packages/e2e/next.config.mjs
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion packages/e2e/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 3 additions & 0 deletions packages/e2e/src/app/app/internals/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ export default function Page() {
<>
<p id="__N">{String(history.state.__N)}</p>
<p id="__NA">{String(history.state.__NA)}</p>
<p id="windowHistorySupport">
{String(process.env.__NEXT_WINDOW_HISTORY_SUPPORT)}
</p>
</>
)
}
19 changes: 11 additions & 8 deletions packages/next-usequerystate/src/update-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`
}
}
10 changes: 10 additions & 0 deletions packages/next-usequerystate/src/useQueryState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,16 @@ export function useQueryState<T = string>(
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) {
Expand Down
Loading

0 comments on commit e911272

Please sign in to comment.