fix(pages-router): format object as navigation urls - #3354
james-elicx wants to merge 13 commits into
Conversation
|
/bigbonk review for issues |
@vinext/cloudflare
create-vinext-app
@vinext/types
vinext
@cloudflare/workers-response-store
commit: |
|
Performance benchmarksCompared 0 improved · 0 regressed · 6 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
|
Posted 1 actionable inline finding. Targeted unit tests passed. |
|
/bigbonk review for issues |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
/bigbonk review for issues |
|
@james-elicx Bonk workflow was cancelled. View workflow run · To retry, trigger Bonk again. |
|
Posted 1 actionable inline finding. Targeted unit tests passed. |
|
Time limit: 8 minutes. Complete the review and post all findings within that limit. |
|
/bigbonk review for issues |
|
Posted 1 actionable inline finding. |
|
Time limit: 8 minutes. Complete the review and post all findings within that limit. |
|
/bigbonk review for issues |
|
Posted 2 actionable inline findings. |
|
Time limit: 8 minutes. Complete the review and post all findings within that limit. |
|
/bigbonk review for issues |
|
Posted 1 actionable inline finding. Targeted URL-object tests passed. |
|
Time limit: 8 minutes. Complete the review and post all findings within that limit. |
|
/bigbonk review for issues |
|
Posted 3 actionable inline findings. |
|
Time limit: 8 minutes. Complete the review and post all findings within that limit. |
|
/bigbonk review for issues |
|
Posted 5 actionable inline findings. |
|
Time limit: 8 minutes. Complete the review and post all findings within that limit. |
|
/bigbonk review for issues |
|
Posted 1 actionable inline finding. No repository changes made. |
|
Time limit: 8 minutes. Complete the review and post all findings within that limit. |
|
/bigbonk review for issues |
|
LGTM! |
|
Time limit: 8 minutes. Complete the review and post all findings within that limit. |
|
/bigbonk review for issues |
|
LGTM! |
|
Time limit: 8 minutes. Complete the review and post all findings within that limit. |
|
/bigbonk review for issues |
|
LGTM! |
2 participants
Closes #3351\n\n## Summary\n\n- accept URL objects for the Pages Router push and replace as argument\n- match Next.js URL-object formatting, WHATWG canonicalization, dynamic/query-only resolution, and same-origin/basePath handling\n- preserve empty query/hash delimiters, tracked asPath semantics, locale-domain masks, and internal double-slash middleware paths\n- preserve dangerous-scheme and external-navigation safety while matching href/as ordering and return values\n- emit the Next.js repeated-slash diagnostic once per invalid string or object argument\n- tolerate runtime null masks like Next.js\n- cover hook push, singleton replace, query/search/hash, history behavior, unsafe schemes, locale/basePath, relative/protocol/authority forms, same-origin boundaries, and external href/as combinations\n\n## Root cause\n\nThe first URL argument was formatted, but object-form as values reached string-only URL handling unchanged. The public declarations also restricted as to strings. Matching prepareUrlAs required formatting the full UrlObject surface, resolving every local object through WHATWG URL semantics against router state, keeping explicit as behavior distinct from href interpolation, and preserving basePath, delimiter, middleware, locale-domain, and external-navigation invariants.\n\nNext.js references:\n- https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/router/router.ts\n- https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/router/utils/format-url.ts\n- https://github.com/vercel/next.js/blob/canary/packages/next/src/client/resolve-href.ts\n\n## Validation\n\n- vp test run tests/shims.test.ts tests/router-javascript-urls.test.ts (1,426 passed)\n- targeted Pages Router browser regressions (2 passed on isolated port 4273)\n- vp check\n- vp run vinext#build\n- node scripts/sync-next-types.mjs --check\n- node scripts/check-shim-types.mjs\n- simplification review: 22 runtime lines and 172 net test-scaffolding lines removed with parity cases preserved