Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ensure history navigates correctly with dynamic routes + basePath #25459

Merged
merged 2 commits into from
May 26, 2021
Merged

Ensure history navigates correctly with dynamic routes + basePath #25459

merged 2 commits into from
May 26, 2021

Conversation

ijjk
Copy link
Member

@ijjk ijjk commented May 25, 2021

This ensures the basePath is correctly re-added to the url after resolving a dynamic route since the url stored in history is expected to already contain the basePath, an additional test has been added to ensure this is working correctly back/forward through history

Bug

  • Related issues linked using fixes #number
  • Integration tests added

Fixes: #25285

@ijjk

This comment has been minimized.

Sign in to view
@ijjk

This comment has been minimized.

Sign in to view
@ijjk
Copy link
Member Author

ijjk commented May 25, 2021

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history Change
buildDuration 15.9s 16s ⚠️ +150ms
buildDurationCached 4.4s 4.5s ⚠️ +111ms
nodeModulesSize 46.7 MB 46.7 MB ⚠️ +282 B
Page Load Tests Overall increase ✓
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.924 2.86 -0.06
/ avg req/sec 854.87 874.27 +19.4
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.729 1.652 -0.08
/error-in-render avg req/sec 1445.77 1513.24 +67.47
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history Change
framework-HASH.js gzip 39.3 kB 39.3 kB ✓
main-HASH.js gzip 19.4 kB 19.4 kB ⚠️ +8 B
webpack-HASH.js gzip 994 B 994 B ✓
Overall change 59.7 kB 59.7 kB ⚠️ +8 B
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history 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/basepath-dynamic-history 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.54 kB 8.54 kB ✓
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history Change
_buildManifest.js gzip 390 B 390 B ✓
Overall change 390 B 390 B ✓
Rendered Page Sizes Overall decrease ✓
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history Change
index.html gzip 561 B 560 B -1 B
link.html gzip 569 B 568 B -1 B
withRouter.html gzip 556 B 556 B ✓
Overall change 1.69 kB 1.68 kB -2 B

Diffs

Diff for main-HASH.js 
@@ -4021,6 +4021,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
 
                                 if (parsed.pathname !== pathname) {
                                   pathname = parsed.pathname;
+                                  parsed.pathname = addBasePath(pathname);
                                   url = (0, _utils.formatWithValidation)(
                                     parsed
                                   );
@@ -4944,6 +4945,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
 
                             if (parsed.pathname !== pathname) {
                               pathname = parsed.pathname;
+                              parsed.pathname = pathname;
                               url = (0, _utils.formatWithValidation)(parsed);
                             }
Diff for index.html 
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/main-aa7354814c85fc15b2e2.js"
+      href="/_next/static/chunks/main-748cc50278e954098517.js"
       as="script"
     />
     <link
@@ -56,7 +56,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/main-aa7354814c85fc15b2e2.js"
+      src="/_next/static/chunks/main-748cc50278e954098517.js"
       async=""
     ></script>
     <script
Diff for link.html 
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/main-aa7354814c85fc15b2e2.js"
+      href="/_next/static/chunks/main-748cc50278e954098517.js"
       as="script"
     />
     <link
@@ -61,7 +61,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/main-aa7354814c85fc15b2e2.js"
+      src="/_next/static/chunks/main-748cc50278e954098517.js"
       async=""
     ></script>
     <script
Diff for withRouter.html 
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/main-aa7354814c85fc15b2e2.js"
+      href="/_next/static/chunks/main-748cc50278e954098517.js"
       as="script"
     />
     <link
@@ -56,7 +56,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/main-aa7354814c85fc15b2e2.js"
+      src="/_next/static/chunks/main-748cc50278e954098517.js"
       async=""
     ></script>
     <script
Serverless Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history Change
buildDuration 18.2s 18.2s ⚠️ +49ms
buildDurationCached 5.5s 5.4s -79ms
nodeModulesSize 46.7 MB 46.7 MB ⚠️ +282 B
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history Change
framework-HASH.js gzip 39.3 kB 39.3 kB ✓
main-HASH.js gzip 19.4 kB 19.4 kB ⚠️ +8 B
webpack-HASH.js gzip 994 B 994 B ✓
Overall change 59.7 kB 59.7 kB ⚠️ +8 B
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history 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/basepath-dynamic-history 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.54 kB 8.54 kB ✓
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history Change
_buildManifest.js gzip 390 B 390 B ✓
Overall change 390 B 390 B ✓
Serverless bundles Overall decrease ✓
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history Change
_error.js 16.9 kB 16.9 kB -2 B
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 -2 B
Webpack 4 Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history Change
buildDuration 14.5s 14.4s -183ms
buildDurationCached 6.1s 5.9s -166ms
nodeModulesSize 46.7 MB 46.7 MB ⚠️ +282 B
Page Load Tests Overall increase ✓
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.932 2.922 -0.01
/ avg req/sec 852.67 855.66 +2.99
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.7 1.665 -0.03
/error-in-render avg req/sec 1470.94 1501.92 +30.98
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history Change
677f882d2ed8..HASH.js gzip 13.3 kB 13.3 kB ⚠️ +7 B
framework.HASH.js gzip 39 kB 39 kB ✓
main-HASH.js gzip 7.25 kB 7.25 kB ✓
webpack-HASH.js gzip 751 B 751 B ✓
Overall change 60.3 kB 60.3 kB ⚠️ +7 B
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history 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/basepath-dynamic-history 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.64 kB 1.64 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.38 kB 9.38 kB ✓
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history Change
_buildManifest.js gzip 420 B 420 B ✓
Overall change 420 B 420 B ✓
Rendered Page Sizes Overall decrease ✓
vercel/next.js canary ijjk/next.js fix/basepath-dynamic-history Change
index.html gzip 614 B 613 B -1 B
link.html gzip 620 B 620 B ✓
withRouter.html gzip 606 B 606 B ✓
Overall change 1.84 kB 1.84 kB -1 B

Diffs

Diff for 677f882d2ed8..c4df.HASH.js 
@@ -2091,6 +2091,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
 
                                 if (parsed.pathname !== pathname) {
                                   pathname = parsed.pathname;
+                                  parsed.pathname = addBasePath(pathname);
                                   url = (0, _utils.formatWithValidation)(
                                     parsed
                                   );
@@ -3012,6 +3013,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
 
                             if (parsed.pathname !== pathname) {
                               pathname = parsed.pathname;
+                              parsed.pathname = pathname;
                               url = (0, _utils.formatWithValidation)(parsed);
                             }
Diff for index.html 
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.26e6c0f15d34a1b1b821.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.e9c74922df35ac7b44e8.js"
       as="script"
     />
     <link
@@ -61,7 +61,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.26e6c0f15d34a1b1b821.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.e9c74922df35ac7b44e8.js"
       async=""
     ></script>
     <script
Diff for link.html 
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.26e6c0f15d34a1b1b821.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.e9c74922df35ac7b44e8.js"
       as="script"
     />
     <link
@@ -66,7 +66,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.26e6c0f15d34a1b1b821.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.e9c74922df35ac7b44e8.js"
       async=""
     ></script>
     <script
Diff for withRouter.html 
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.26e6c0f15d34a1b1b821.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.e9c74922df35ac7b44e8.js"
       as="script"
     />
     <link
@@ -61,7 +61,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.26e6c0f15d34a1b1b821.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.e9c74922df35ac7b44e8.js"
       async=""
     ></script>
     <script
Commit: 97838da

divmain
divmain approved these changes May 26, 2021
View reviewed changes
Copy link
Contributor

@divmain divmain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! Thanks for the quick turn-around!

@kodiakhq kodiakhq bot merged commit 08dce4b into vercel:canary May 26, 2021
@ijjk ijjk deleted the fix/basepath-dynamic-history branch May 26, 2021 13:23
@sync-by-unito sync-by-unito bot mentioned this pull request Jun 16, 2021
Closed
flybayer pushed a commit to blitz-js/next.js that referenced this pull request Jun 16, 2021
@ijjk
Ensure history navigates correctly with dynamic routes + basePath (ve…
1b74292 
…rcel#25459)

This ensures the `basePath` is correctly re-added to the `url` after resolving a dynamic route since the `url` stored in history is expected to already contain the `basePath`, an additional test has been added to ensure this is working correctly back/forward through history

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added

Fixes: vercel#25285
@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@divmain divmain divmain approved these changes

@lfades lfades Awaiting requested review from lfades

@shuding shuding Awaiting requested review from shuding

@timneutkens timneutkens Awaiting requested review from timneutkens

Assignees
No one assigned
Labels
type: next
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

router.back() not working when going back between two dynamic routes
2 participants
@ijjk @divmain