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

PepijnSenders · 2021-05-26T17:33:52Z

Description

In this PR going back on a rewritten dynamic page (with getServerSideProps) with dynamic params is broken. I added the integration test which is currently broken to showcase the issue:

Will add the fix to this PR as well.

The fix for most pages was merged here, this did not include dynamic pages yet though. I set _shouldResolveHref to true in the state options of the initial route when as !== href.

Bug

Related issues linked using fixes #25490
Integration tests added

PepijnSenders · 2021-05-26T18:04:08Z

test/integration/rewrite-with-browser-history/next.config.js

+    return [
+      {
+        source: '/:pagePrefix/:path*',
+        destination: '/dynamic-page/:path*',


This is where it goes wrong I guess, the first param is not defined in the second path, and the [[...param]] then doesn't get defined and the router thinks it can't match the route.

Before when we resolved the rewrites on these URL's it did work though because it will rematch this and figure out that the [[...param]] is actually in that URL.

ijjk · 2021-06-01T20:22:55Z

Re-opening to continue #25666 (comment)

PepijnSenders · 2021-06-01T20:57:02Z

@ijjk this looks good. Also tested it in our app and the test is green 👍

ijjk · 2021-06-01T21:12:16Z

Stats from current PR

Default Server Mode (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	Change
buildDuration	13.6s	13.9s	⚠️ +304ms
buildDurationCached	3.9s	3.8s	-49ms
nodeModulesSize	46.7 MB	46.7 MB	⚠️ +273 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.375	2.349	-0.03
/ avg req/sec	1052.44	1064.14	+11.7
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.267	1.226	-0.04
/error-in-render avg req/sec	1973.65	2039.23	+65.58

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

	vercel/next.js canary	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	Change
framework-HASH.js gzip	39.3 kB	39.3 kB	✓
main-HASH.js gzip	19.4 kB	19.5 kB	⚠️ +19 B
webpack-HASH.js gzip	804 B	804 B	✓
Overall change	59.5 kB	59.6 kB	⚠️ +19 B

Legacy Client Bundles (polyfills)

	vercel/next.js canary	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	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	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	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	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	Change
_buildManifest.js gzip	392 B	392 B	✓
Overall change	392 B	392 B	✓

Rendered Page Sizes Overall increase ⚠️

	vercel/next.js canary	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	Change
index.html gzip	558 B	560 B	⚠️ +2 B
link.html gzip	566 B	568 B	⚠️ +2 B
withRouter.html gzip	554 B	556 B	⚠️ +2 B
Overall change	1.68 kB	1.68 kB	⚠️ +6 B

Diffs

Diff for main-HASH.js

@@ -3663,6 +3663,10 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
             if (_as.substr(0, 2) !== "//") {
               // in order for `e.state` to work on the `onpopstate` event
               // we have to register the initial route upon initialization
+              var options = {
+                locale: locale
+              };
+              options._shouldResolveHref = _as !== _pathname;
               this.changeState(
                 "replaceState",
                 (0, _utils.formatWithValidation)({
@@ -3670,9 +3674,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
                   query: _query
                 }),
                 (0, _utils.getURL)(),
-                {
-                  locale: locale
-                }
+                options
               );
             }

Diff for index.html

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

Diff for link.html

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

Diff for withRouter.html

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

Serverless Mode (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	Change
buildDuration	16.5s	16s	-468ms
buildDurationCached	5s	5s	⚠️ +1ms
nodeModulesSize	46.7 MB	46.7 MB	⚠️ +273 B

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

	vercel/next.js canary	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	Change
framework-HASH.js gzip	39.3 kB	39.3 kB	✓
main-HASH.js gzip	19.4 kB	19.5 kB	⚠️ +19 B
webpack-HASH.js gzip	804 B	804 B	✓
Overall change	59.5 kB	59.6 kB	⚠️ +19 B

Legacy Client Bundles (polyfills)

	vercel/next.js canary	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	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	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	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	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	Change
_buildManifest.js gzip	392 B	392 B	✓
Overall change	392 B	392 B	✓

Serverless bundles

	vercel/next.js canary	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	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	✓
link.js	17.4 kB	17.4 kB	✓
routerDirect.js	17.4 kB	17.4 kB	✓
withRouter.js	17.4 kB	17.4 kB	✓
Overall change	107 kB	107 kB	✓

Webpack 4 Mode (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	Change
buildDuration	12.7s	12.6s	-115ms
buildDurationCached	5.5s	5.3s	-165ms
nodeModulesSize	46.7 MB	46.7 MB	⚠️ +273 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.466	2.5	⚠️ +0.03
/ avg req/sec	1013.69	1000	⚠️ -13.69
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.323	1.291	-0.03
/error-in-render avg req/sec	1889.59	1936.09	+46.5

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

	vercel/next.js canary	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	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	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	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	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	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	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	Change
_buildManifest.js gzip	420 B	420 B	✓
Overall change	420 B	420 B	✓

Rendered Page Sizes Overall increase ⚠️

	vercel/next.js canary	PepijnSenders/next.js feature/fix-rewrite-and-dynamic-params	Change
index.html gzip	614 B	614 B	✓
link.html gzip	620 B	620 B	✓
withRouter.html gzip	607 B	608 B	⚠️ +1 B
Overall change	1.84 kB	1.84 kB	⚠️ +1 B

Diffs

Diff for 677f882d2ed8..c4df.HASH.js

@@ -1734,6 +1734,10 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
             if (_as.substr(0, 2) !== "//") {
               // in order for `e.state` to work on the `onpopstate` event
               // we have to register the initial route upon initialization
+              var options = {
+                locale: locale
+              };
+              options._shouldResolveHref = _as !== _pathname;
               this.changeState(
                 "replaceState",
                 (0, _utils.formatWithValidation)({
@@ -1741,9 +1745,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
                   query: _query
                 }),
                 (0, _utils.getURL)(),
-                {
-                  locale: locale
-                }
+                options
               );
             }

Diff for index.html

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

Diff for link.html

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

Diff for withRouter.html

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

Commit: 643e800

ijjk

Looks good, thanks for helping track this down!

…vercel#25495) * Fix rewrite and dynamic params * set _shouldResolveHref to true when initializing client state * add window.beforeNav check * fix broken path * fix build-output tests * remove yarn.lock changes

PepijnSenders requested review from divmain, ijjk, lfades, shuding and timneutkens as code owners May 26, 2021 17:33

ijjk added the type: next label May 26, 2021

Fix rewrite and dynamic params

bd7a4b3

PepijnSenders force-pushed the feature/fix-rewrite-and-dynamic-params branch from f9590e5 to bd7a4b3 Compare May 26, 2021 17:36

PepijnSenders mentioned this pull request May 26, 2021

Back button for rewritten page broken in last version #25490

Closed

Merge branch 'canary' into feature/fix-rewrite-and-dynamic-params

58ee309

This comment has been minimized.

Sign in to view

PepijnSenders commented May 26, 2021

View reviewed changes

ijjk mentioned this pull request Jun 1, 2021

Ensure rewrites are resolved correctly through history #25666

Merged

2 tasks

ijjk closed this in #25666 Jun 1, 2021

Merge branch 'canary' into feature/fix-rewrite-and-dynamic-params

0503e34

ijjk reopened this Jun 1, 2021

set _shouldResolveHref to true when initializing client state

1e7d161

This comment has been minimized.

Sign in to view

PepijnSenders added 2 commits June 1, 2021 16:53

add window.beforeNav check

57aca30

fix broken path

3a01bea

This comment has been minimized.

Sign in to view

PepijnSenders added 2 commits June 1, 2021 16:57

fix build-output tests

66cfd98

remove yarn.lock changes

643e800

ijjk approved these changes Jun 1, 2021

View reviewed changes

ijjk changed the title ~~Fix rewrite and dynamic params~~ Fix rewrite and dynamic params on navigating to initial history entry Jun 1, 2021

ijjk merged commit 9c3780f into vercel:canary Jun 1, 2021

PepijnSenders deleted the feature/fix-rewrite-and-dynamic-params branch June 2, 2021 01:33

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

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

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

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

PepijnSenders commented May 26, 2021 •

edited

This comment has been minimized.

This comment has been minimized.

PepijnSenders May 26, 2021 •

edited

ijjk commented Jun 1, 2021

This comment has been minimized.

This comment has been minimized.

PepijnSenders commented Jun 1, 2021

ijjk commented Jun 1, 2021

Diffs

Diffs

ijjk left a comment

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

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

Conversation

PepijnSenders commented May 26, 2021 • edited

Description

Bug

This comment has been minimized.

This comment has been minimized.

PepijnSenders May 26, 2021 • edited

Choose a reason for hiding this comment

ijjk commented Jun 1, 2021

This comment has been minimized.

This comment has been minimized.

PepijnSenders commented Jun 1, 2021

ijjk commented Jun 1, 2021

Stats from current PR

Diffs

Diffs

ijjk left a comment

Choose a reason for hiding this comment

PepijnSenders commented May 26, 2021 •

edited

PepijnSenders May 26, 2021 •

edited