Ensure rewrites are resolved correctly through history #25666

ijjk · 2021-06-01T01:18:07Z

This ensures we resolve rewrites correctly through history since we store the end url and as although when rewriting to a dynamic page the resolvedAs is needed to match against. An additional test has been added to ensure this works as expected.

Bug

Related issues linked using fixes #number
Integration tests added

Fixes: #25490
Closes: #25495

ijjk · 2021-06-01T17:27:16Z

Stats from current PR

Default Server Mode (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
buildDuration	16s	16.1s	⚠️ +140ms
buildDurationCached	4.4s	4.1s	-270ms
nodeModulesSize	46.7 MB	46.7 MB	⚠️ +254 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.992	2.885	-0.11
/ avg req/sec	835.5	866.55	+31.05
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.871	1.837	-0.03
/error-in-render avg req/sec	1336.11	1361.21	+25.1

Client Bundles (main, webpack, commons) Overall increase ⚠️

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
framework-HASH.js gzip	39.3 kB	39.3 kB	✓
main-HASH.js gzip	19.4 kB	19.4 kB	⚠️ +11 B
webpack-HASH.js gzip	804 B	804 B	✓
Overall change	59.5 kB	59.5 kB	⚠️ +11 B

Legacy Client Bundles (polyfills)

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
polyfills-HASH.js gzip	31.1 kB	31.1 kB	✓
Overall change	31.1 kB	31.1 kB	✓

Client Pages

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
_app-HASH.js gzip	1.02 kB	1.02 kB	✓
_error-HASH.js gzip	3.06 kB	3.06 kB	✓
amp-HASH.js gzip	526 B	526 B	✓
css-HASH.js gzip	334 B	334 B	✓
hooks-HASH.js gzip	890 B	890 B	✓
index-HASH.js gzip	262 B	262 B	✓
link-HASH.js gzip	1.65 kB	1.65 kB	✓
routerDirect..HASH.js gzip	331 B	331 B	✓
withRouter-HASH.js gzip	329 B	329 B	✓
bb14e60e810b..30f.css gzip	125 B	125 B	✓
Overall change	8.53 kB	8.53 kB	✓

Client Build Manifests

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
_buildManifest.js gzip	392 B	392 B	✓
Overall change	392 B	392 B	✓

Rendered Page Sizes Overall decrease ✓

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
index.html gzip	561 B	558 B	-3 B
link.html gzip	569 B	566 B	-3 B
withRouter.html gzip	557 B	554 B	-3 B
Overall change	1.69 kB	1.68 kB	-9 B

Diffs

Diff for main-HASH.js

@@ -3815,7 +3815,10 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
                             return _context.abrupt("return", false);
 
                           case 3:
-                            shouldResolveHref = url === as || options._h; // for static pages with query params in the URL we delay
+                            shouldResolveHref =
+                              url === as ||
+                              options._h ||
+                              options._shouldResolveHref; // for static pages with query params in the URL we delay
                             // marking the router ready until after the query is updated
 
                             if (options._h) {
@@ -4014,6 +4017,8 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
                               : pathname;
 
                             if (shouldResolveHref && pathname !== "/_error") {
+                              options._shouldResolveHref = true;
+
                               if (false) {
                               } else {
                                 parsed.pathname = resolveDynamicRoute(

Diff for index.html

@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/main-400baffbe1ec6a36a6e9.js"
+      href="/_next/static/chunks/main-526073a735189d761c47.js"
       as="script"
     />
     <link
@@ -56,7 +56,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/main-400baffbe1ec6a36a6e9.js"
+      src="/_next/static/chunks/main-526073a735189d761c47.js"
       async=""
     ></script>
     <script

Diff for link.html

@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/main-400baffbe1ec6a36a6e9.js"
+      href="/_next/static/chunks/main-526073a735189d761c47.js"
       as="script"
     />
     <link
@@ -61,7 +61,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/main-400baffbe1ec6a36a6e9.js"
+      src="/_next/static/chunks/main-526073a735189d761c47.js"
       async=""
     ></script>
     <script

Diff for withRouter.html

@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/main-400baffbe1ec6a36a6e9.js"
+      href="/_next/static/chunks/main-526073a735189d761c47.js"
       as="script"
     />
     <link
@@ -56,7 +56,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/main-400baffbe1ec6a36a6e9.js"
+      src="/_next/static/chunks/main-526073a735189d761c47.js"
       async=""
     ></script>
     <script

Serverless Mode (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
buildDuration	17.5s	16.9s	-635ms
buildDurationCached	5.2s	5.1s	-89ms
nodeModulesSize	46.7 MB	46.7 MB	⚠️ +254 B

Client Bundles (main, webpack, commons) Overall increase ⚠️

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
framework-HASH.js gzip	39.3 kB	39.3 kB	✓
main-HASH.js gzip	19.4 kB	19.4 kB	⚠️ +11 B
webpack-HASH.js gzip	804 B	804 B	✓
Overall change	59.5 kB	59.5 kB	⚠️ +11 B

Legacy Client Bundles (polyfills)

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
polyfills-HASH.js gzip	31.1 kB	31.1 kB	✓
Overall change	31.1 kB	31.1 kB	✓

Client Pages

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
_app-HASH.js gzip	1.02 kB	1.02 kB	✓
_error-HASH.js gzip	3.06 kB	3.06 kB	✓
amp-HASH.js gzip	526 B	526 B	✓
css-HASH.js gzip	334 B	334 B	✓
hooks-HASH.js gzip	890 B	890 B	✓
index-HASH.js gzip	262 B	262 B	✓
link-HASH.js gzip	1.65 kB	1.65 kB	✓
routerDirect..HASH.js gzip	331 B	331 B	✓
withRouter-HASH.js gzip	329 B	329 B	✓
bb14e60e810b..30f.css gzip	125 B	125 B	✓
Overall change	8.53 kB	8.53 kB	✓

Client Build Manifests

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
_buildManifest.js gzip	392 B	392 B	✓
Overall change	392 B	392 B	✓

Serverless bundles Overall increase ⚠️

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
_error.js	16.9 kB	16.9 kB	✓
404.html	2.42 kB	2.42 kB	✓
500.html	2.41 kB	2.41 kB	✓
amp.amp.html	10.8 kB	10.8 kB	✓
amp.html	1.61 kB	1.61 kB	✓
css.html	1.79 kB	1.79 kB	✓
hooks.html	1.67 kB	1.67 kB	✓
index.js	17.2 kB	17.2 kB	⚠️ +2 B
link.js	17.4 kB	17.4 kB	⚠️ +2 B
routerDirect.js	17.4 kB	17.4 kB	✓
withRouter.js	17.4 kB	17.4 kB	✓
Overall change	107 kB	107 kB	⚠️ +4 B

Webpack 4 Mode (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
buildDuration	13.4s	13.4s	⚠️ +6ms
buildDurationCached	5.9s	5.8s	-165ms
nodeModulesSize	46.7 MB	46.7 MB	⚠️ +254 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.907	2.731	-0.18
/ avg req/sec	859.88	915.39	+55.51
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.744	1.708	-0.04
/error-in-render avg req/sec	1433.69	1463.35	+29.66

Client Bundles (main, webpack, commons) Overall increase ⚠️

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
677f882d2ed8..HASH.js gzip	13.3 kB	13.3 kB	⚠️ +12 B
framework.HASH.js gzip	39 kB	39 kB	✓
main-HASH.js gzip	7.26 kB	7.26 kB	✓
webpack-HASH.js gzip	751 B	751 B	✓
Overall change	60.3 kB	60.3 kB	⚠️ +12 B

Legacy Client Bundles (polyfills)

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
polyfills-HASH.js gzip	31.3 kB	31.3 kB	✓
Overall change	31.3 kB	31.3 kB	✓

Client Pages

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
_app-HASH.js gzip	1.28 kB	1.28 kB	✓
_error-HASH.js gzip	3.74 kB	3.74 kB	✓
amp-HASH.js gzip	536 B	536 B	✓
css-HASH.js gzip	339 B	339 B	✓
hooks-HASH.js gzip	887 B	887 B	✓
index-HASH.js gzip	227 B	227 B	✓
link-HASH.js gzip	1.63 kB	1.63 kB	✓
routerDirect..HASH.js gzip	303 B	303 B	✓
withRouter-HASH.js gzip	302 B	302 B	✓
e025d2764813..52f.css gzip	125 B	125 B	✓
Overall change	9.37 kB	9.37 kB	✓

Client Build Manifests

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
_buildManifest.js gzip	420 B	420 B	✓
Overall change	420 B	420 B	✓

Rendered Page Sizes Overall increase ⚠️

	vercel/next.js canary	ijjk/next.js fix/history-rewrite-resolve	Change
index.html gzip	612 B	614 B	⚠️ +2 B
link.html gzip	620 B	620 B	✓
withRouter.html gzip	607 B	607 B	✓
Overall change	1.84 kB	1.84 kB	⚠️ +2 B

Diffs

Diff for 677f882d2ed8..c4df.HASH.js

@@ -1886,7 +1886,10 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
                             return _context.abrupt("return", false);
 
                           case 3:
-                            shouldResolveHref = url === as || options._h; // for static pages with query params in the URL we delay
+                            shouldResolveHref =
+                              url === as ||
+                              options._h ||
+                              options._shouldResolveHref; // for static pages with query params in the URL we delay
                             // marking the router ready until after the query is updated
 
                             if (options._h) {
@@ -2085,6 +2088,8 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
                               : pathname;
 
                             if (shouldResolveHref && pathname !== "/_error") {
+                              options._shouldResolveHref = true;
+
                               if (false) {
                               } else {
                                 parsed.pathname = resolveDynamicRoute(

Diff for index.html

@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.bb905fc653e2e79a43a3.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.966674eb01503d915283.js"
       as="script"
     />
     <link
@@ -61,7 +61,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.bb905fc653e2e79a43a3.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.966674eb01503d915283.js"
       async=""
     ></script>
     <script

Diff for link.html

@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.bb905fc653e2e79a43a3.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.966674eb01503d915283.js"
       as="script"
     />
     <link
@@ -66,7 +66,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.bb905fc653e2e79a43a3.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.966674eb01503d915283.js"
       async=""
     ></script>
     <script

Diff for withRouter.html

@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.bb905fc653e2e79a43a3.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.966674eb01503d915283.js"
       as="script"
     />
     <link
@@ -61,7 +61,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.bb905fc653e2e79a43a3.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.966674eb01503d915283.js"
       async=""
     ></script>
     <script

Commit: e78e45b

PepijnSenders · 2021-06-01T19:35:36Z

@ijjk this doesn't work when the first page is the rewritten page and you want to go back to that page. Will make a PR to try to address that.

ijjk · 2021-06-01T20:04:40Z

@PepijnSenders sounds like we need to set the new _shouldResolveHref option on this line when the initial page is a dynamic route and the asPath doesn't match the route to handle that case 🤔

PepijnSenders · 2021-06-01T20:21:48Z

@ijjk that's what I was thinking, should we check if as !== path there? That might rule out already resolved URL's.

In the meantime I'm trying to write a proper test case for this and comparing an integration test and our app, will let you know if I find something there.

ijjk · 2021-06-01T20:28:02Z

@PepijnSenders yeah it looks like if as !== pathname on initial load we need to resolve the href since that can only occur when either a dynamic route is being used or a rewrite is being used.

I re-opened #25495 so we can apply this in that PR with the additional test

* Ensure rewrites are resolved correctly through history * update test

Ensure rewrites are resolved correctly through history

a507e06

ijjk added the type: next label Jun 1, 2021

This comment has been minimized.

Sign in to view

update test

67dc105

This comment has been minimized.

Sign in to view

ijjk marked this pull request as ready for review June 1, 2021 02:17

ijjk requested review from divmain, lfades, shuding and timneutkens as code owners June 1, 2021 02:17

Merge branch 'canary' into fix/history-rewrite-resolve

e1b12c6

This comment has been minimized.

Sign in to view

timneutkens approved these changes Jun 1, 2021

View reviewed changes

Merge branch 'canary' into fix/history-rewrite-resolve

e78e45b

ijjk merged commit 5476827 into vercel:canary Jun 1, 2021

PepijnSenders mentioned this pull request Jun 1, 2021

Back button for rewritten page broken in last version #25490

Closed

ijjk deleted the fix/history-rewrite-resolve branch June 1, 2021 19:58

ijjk mentioned this pull request Jun 1, 2021

Fix rewrite and dynamic params on navigating to initial history entry #25495

Merged

2 tasks

ijjk mentioned this pull request Jun 14, 2021

Error occurred when using dynamic routing and Rewrite together #26074

Closed

sync-by-unito bot mentioned this pull request Jun 16, 2021

chore(deps): bump next from 10.1.2 to 11.0.0 filecoin-project/slate#793

Closed

flybayer pushed a commit to blitz-js/next.js that referenced this pull request Jun 16, 2021

Ensure rewrites are resolved correctly through history (vercel#25666)

715f883

* Ensure rewrites are resolved correctly through history * update test

vercel locked as resolved and limited conversation to collaborators Jan 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure rewrites are resolved correctly through history #25666

Ensure rewrites are resolved correctly through history #25666

ijjk commented Jun 1, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Jun 1, 2021

Diffs

Diffs

PepijnSenders commented Jun 1, 2021

ijjk commented Jun 1, 2021

PepijnSenders commented Jun 1, 2021 •

edited

ijjk commented Jun 1, 2021

Ensure rewrites are resolved correctly through history #25666

Ensure rewrites are resolved correctly through history #25666

Conversation

ijjk commented Jun 1, 2021

Bug

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Jun 1, 2021

Stats from current PR

Diffs

Diffs

PepijnSenders commented Jun 1, 2021

ijjk commented Jun 1, 2021

PepijnSenders commented Jun 1, 2021 • edited

ijjk commented Jun 1, 2021

PepijnSenders commented Jun 1, 2021 •

edited