-
Notifications
You must be signed in to change notification settings - Fork 27.6k
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.
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
fix: use a client-side navigation when redirecting to a rewriten URL #25990
fix: use a client-side navigation when redirecting to a rewriten URL #25990
Conversation
test/integration/redirect-getserversideprops-rewrites/test/index.test.js
Outdated
Show resolved
Hide resolved
test/integration/redirect-getserversideprops-rewrites/test/index.test.js
Outdated
Show resolved
Hide resolved
80f1638
to
066846c
Compare
test/integration/gssp-redirect-with-rewrites/test/index.test.js
Outdated
Show resolved
Hide resolved
test/integration/gssp-redirect-with-rewrites/test/index.test.js
Outdated
Show resolved
Hide resolved
test/integration/gssp-redirect-with-rewrites/test/index.test.js
Outdated
Show resolved
Hide resolved
}) | ||
afterAll(() => killApp(context.server)) | ||
|
||
it('should use a client-side navigation for a rewritten URL', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a similar test but instead ensure the /unknown-route
being returned as a redirect does do a full navigation instead of a client navigation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done ! Thanks for your suggestions 😄
For some reason the following tests are failing on the pipeline, but not on my computer :
I'm changing the |
This comment has been minimized.
This comment has been minimized.
Co-authored-by: JJ Kasper <jj@jjsweb.site>
Co-authored-by: JJ Kasper <jj@jjsweb.site>
Co-authored-by: JJ Kasper <jj@jjsweb.site>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…rside-props-with-rewrites
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Stats from current PRDefault Server Mode (Decrease detected ✓)General Overall decrease ✓
Page Load Tests Overall decrease
|
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 2.302 | 2.275 | -0.03 |
/ avg req/sec | 1085.93 | 1098.71 | +12.78 |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.264 | 1.295 | |
/error-in-render avg req/sec | 1978.08 | 1930.77 |
Client Bundles (main, webpack, commons) Overall decrease ✓
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
framework-HASH.js gzip | 42 kB | 42 kB | ✓ |
main-HASH.js gzip | 20.2 kB | 20.2 kB | -13 B |
webpack-HASH.js gzip | 804 B | 804 B | ✓ |
Overall change | 63 kB | 63 kB | -13 B |
Legacy Client Bundles (polyfills)
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | 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 | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
_app-HASH.js gzip | 801 B | 801 B | ✓ |
_error-HASH.js gzip | 3.17 kB | 3.17 kB | ✓ |
amp-HASH.js gzip | 527 B | 527 B | ✓ |
css-HASH.js gzip | 329 B | 329 B | ✓ |
hooks-HASH.js gzip | 903 B | 903 B | ✓ |
index-HASH.js gzip | 263 B | 263 B | ✓ |
link-HASH.js gzip | 1.65 kB | 1.65 kB | ✓ |
routerDirect..HASH.js gzip | 322 B | 322 B | ✓ |
withRouter-HASH.js gzip | 320 B | 320 B | ✓ |
bb14e60e810b..30f.css gzip | 125 B | 125 B | ✓ |
Overall change | 8.41 kB | 8.41 kB | ✓ |
Client Build Manifests
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
_buildManifest.js gzip | 391 B | 391 B | ✓ |
Overall change | 391 B | 391 B | ✓ |
Rendered Page Sizes Overall decrease ✓
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
index.html gzip | 523 B | 522 B | -1 B |
link.html gzip | 535 B | 534 B | -1 B |
withRouter.html gzip | 514 B | 515 B | |
Overall change | 1.57 kB | 1.57 kB | -1 B |
Diffs
Diff for main-HASH.js
@@ -4406,14 +4406,14 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
(__N_SSP = _routeInfo.__N_SSP); // handle redirect on client-transition
if (!((__N_SSG || __N_SSP) && props)) {
- _context.next = 107;
+ _context.next = 106;
break;
}
if (
!(props.pageProps && props.pageProps.__N_REDIRECT)
) {
- _context.next = 93;
+ _context.next = 92;
break;
}
@@ -4422,7 +4422,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
// it's not
if (!destination.startsWith("/")) {
- _context.next = 91;
+ _context.next = 90;
break;
}
@@ -4432,12 +4432,6 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
parsedHref.pathname,
pages
);
-
- if (!pages.includes(parsedHref.pathname)) {
- _context.next = 91;
- break;
- }
-
(_prepareUrlAs3 = prepareUrlAs(
this,
destination,
@@ -4450,37 +4444,37 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
this.change(method, newUrl, newAs, options)
);
- case 91:
+ case 90:
window.location.href = destination;
return _context.abrupt(
"return",
new Promise(function() {})
);
- case 93:
+ case 92:
this.isPreview = !!props.__N_PREVIEW; // handle SSG data 404
if (!(props.notFound === SSG_DATA_NOT_FOUND)) {
- _context.next = 107;
+ _context.next = 106;
break;
}
- _context.prev = 95;
- _context.next = 98;
+ _context.prev = 94;
+ _context.next = 97;
return this.fetchComponent("/404");
- case 98:
+ case 97:
notFoundRoute = "/404";
- _context.next = 104;
+ _context.next = 103;
break;
- case 101:
- _context.prev = 101;
- _context.t1 = _context["catch"](95);
+ case 100:
+ _context.prev = 100;
+ _context.t1 = _context["catch"](94);
notFoundRoute = "/_error";
- case 104:
- _context.next = 106;
+ case 103:
+ _context.next = 105;
return this.getRouteInfo(
notFoundRoute,
notFoundRoute,
@@ -4492,10 +4486,10 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
}
);
- case 106:
+ case 105:
routeInfo = _context.sent;
- case 107:
+ case 106:
Router.events.emit(
"beforeHistoryChange",
as,
@@ -4536,7 +4530,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
y: 0
}
: null;
- _context.next = 116;
+ _context.next = 115;
return this.set(
route,
pathname,
@@ -4549,9 +4543,9 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
else throw e;
});
- case 116:
+ case 115:
if (!error) {
- _context.next = 119;
+ _context.next = 118;
break;
}
@@ -4563,7 +4557,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
);
throw error;
- case 119:
+ case 118:
if (false) {
}
@@ -4574,21 +4568,21 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
);
return _context.abrupt("return", true);
- case 124:
- _context.prev = 124;
+ case 123:
+ _context.prev = 123;
_context.t2 = _context["catch"](77);
if (!_context.t2.cancelled) {
- _context.next = 128;
+ _context.next = 127;
break;
}
return _context.abrupt("return", false);
- case 128:
+ case 127:
throw _context.t2;
- case 129:
+ case 128:
case "end":
return _context.stop();
}
@@ -4598,8 +4592,8 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
this,
[
[36, 46],
- [77, 124],
- [95, 101]
+ [77, 123],
+ [94, 100]
]
);
})
Diff for index.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-bef403aced749a048de0.js"
+ src="/_next/static/chunks/main-ab0242f2a2175939d500.js"
defer=""
></script>
<script
Diff for link.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-bef403aced749a048de0.js"
+ src="/_next/static/chunks/main-ab0242f2a2175939d500.js"
defer=""
></script>
<script
Diff for withRouter.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-bef403aced749a048de0.js"
+ src="/_next/static/chunks/main-ab0242f2a2175939d500.js"
defer=""
></script>
<script
Serverless Mode (Decrease detected ✓)
General Overall decrease ✓
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
buildDuration | 14.6s | 14.7s | |
buildDurationCached | 4.2s | 4.2s | -58ms |
nodeModulesSize | 46.4 MB | 46.4 MB | -193 B |
Client Bundles (main, webpack, commons) Overall decrease ✓
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
framework-HASH.js gzip | 42 kB | 42 kB | ✓ |
main-HASH.js gzip | 20.2 kB | 20.2 kB | -13 B |
webpack-HASH.js gzip | 804 B | 804 B | ✓ |
Overall change | 63 kB | 63 kB | -13 B |
Legacy Client Bundles (polyfills)
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | 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 | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
_app-HASH.js gzip | 801 B | 801 B | ✓ |
_error-HASH.js gzip | 3.17 kB | 3.17 kB | ✓ |
amp-HASH.js gzip | 527 B | 527 B | ✓ |
css-HASH.js gzip | 329 B | 329 B | ✓ |
hooks-HASH.js gzip | 903 B | 903 B | ✓ |
index-HASH.js gzip | 263 B | 263 B | ✓ |
link-HASH.js gzip | 1.65 kB | 1.65 kB | ✓ |
routerDirect..HASH.js gzip | 322 B | 322 B | ✓ |
withRouter-HASH.js gzip | 320 B | 320 B | ✓ |
bb14e60e810b..30f.css gzip | 125 B | 125 B | ✓ |
Overall change | 8.41 kB | 8.41 kB | ✓ |
Client Build Manifests
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
_buildManifest.js gzip | 391 B | 391 B | ✓ |
Overall change | 391 B | 391 B | ✓ |
Serverless bundles Overall decrease ✓
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
_error.js | 16.9 kB | 16.9 kB | ✓ |
404.html | 1.98 kB | 1.98 kB | ✓ |
500.html | 1.96 kB | 1.96 kB | ✓ |
amp.amp.html | 10.8 kB | 10.8 kB | ✓ |
amp.html | 1.17 kB | 1.17 kB | ✓ |
css.html | 1.35 kB | 1.35 kB | ✓ |
hooks.html | 1.23 kB | 1.23 kB | ✓ |
index.js | 17.2 kB | 17.2 kB | ✓ |
link.js | 17.5 kB | 17.5 kB | ✓ |
routerDirect.js | 17.3 kB | 17.3 kB | -2 B |
withRouter.js | 17.3 kB | 17.3 kB | ✓ |
Overall change | 105 kB | 105 kB | -2 B |
Webpack 4 Mode (Decrease detected ✓)
General Overall decrease ✓
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
buildDuration | 11.6s | 11.4s | -176ms |
buildDurationCached | 4.6s | 4.4s | -199ms |
nodeModulesSize | 46.4 MB | 46.4 MB | -193 B |
Page Load Tests Overall decrease ⚠️
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 2.376 | 2.38 | 0 |
/ avg req/sec | 1052.25 | 1050.2 | |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.301 | 1.35 | |
/error-in-render avg req/sec | 1921.17 | 1852.14 |
Client Bundles (main, webpack, commons) Overall decrease ✓
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
677f882d2ed8..HASH.js gzip | 13.4 kB | 13.3 kB | -11 B |
framework.HASH.js gzip | 41.8 kB | 41.8 kB | ✓ |
main-HASH.js gzip | 7.99 kB | 7.99 kB | ✓ |
webpack-HASH.js gzip | 751 B | 751 B | ✓ |
Overall change | 63.9 kB | 63.8 kB | -11 B |
Legacy Client Bundles (polyfills)
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | 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 | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.07 kB | 1.07 kB | ✓ |
_error-HASH.js gzip | 3.84 kB | 3.84 kB | ✓ |
amp-HASH.js gzip | 536 B | 536 B | ✓ |
css-HASH.js gzip | 333 B | 333 B | ✓ |
hooks-HASH.js gzip | 910 B | 910 B | ✓ |
index-HASH.js gzip | 227 B | 227 B | ✓ |
link-HASH.js gzip | 1.64 kB | 1.64 kB | ✓ |
routerDirect..HASH.js gzip | 295 B | 295 B | ✓ |
withRouter-HASH.js gzip | 292 B | 292 B | ✓ |
e025d2764813..52f.css gzip | 125 B | 125 B | ✓ |
Overall change | 9.28 kB | 9.28 kB | ✓ |
Client Build Manifests
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
_buildManifest.js gzip | 420 B | 420 B | ✓ |
Overall change | 420 B | 420 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | antoinechalifour/next.js redirect-getserverside-props-with-rewrites | Change | |
---|---|---|---|
index.html gzip | 568 B | 568 B | ✓ |
link.html gzip | 579 B | 579 B | ✓ |
withRouter.html gzip | 559 B | 559 B | ✓ |
Overall change | 1.71 kB | 1.71 kB | ✓ |
Diffs
Diff for 677f882d2ed8..c4df.HASH.js
@@ -2252,14 +2252,14 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
(__N_SSP = _routeInfo.__N_SSP); // handle redirect on client-transition
if (!((__N_SSG || __N_SSP) && props)) {
- _context.next = 107;
+ _context.next = 106;
break;
}
if (
!(props.pageProps && props.pageProps.__N_REDIRECT)
) {
- _context.next = 93;
+ _context.next = 92;
break;
}
@@ -2268,7 +2268,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
// it's not
if (!destination.startsWith("/")) {
- _context.next = 91;
+ _context.next = 90;
break;
}
@@ -2278,12 +2278,6 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
parsedHref.pathname,
pages
);
-
- if (!pages.includes(parsedHref.pathname)) {
- _context.next = 91;
- break;
- }
-
(_prepareUrlAs3 = prepareUrlAs(
this,
destination,
@@ -2296,37 +2290,37 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
this.change(method, newUrl, newAs, options)
);
- case 91:
+ case 90:
window.location.href = destination;
return _context.abrupt(
"return",
new Promise(function() {})
);
- case 93:
+ case 92:
this.isPreview = !!props.__N_PREVIEW; // handle SSG data 404
if (!(props.notFound === SSG_DATA_NOT_FOUND)) {
- _context.next = 107;
+ _context.next = 106;
break;
}
- _context.prev = 95;
- _context.next = 98;
+ _context.prev = 94;
+ _context.next = 97;
return this.fetchComponent("/404");
- case 98:
+ case 97:
notFoundRoute = "/404";
- _context.next = 104;
+ _context.next = 103;
break;
- case 101:
- _context.prev = 101;
- _context.t1 = _context["catch"](95);
+ case 100:
+ _context.prev = 100;
+ _context.t1 = _context["catch"](94);
notFoundRoute = "/_error";
- case 104:
- _context.next = 106;
+ case 103:
+ _context.next = 105;
return this.getRouteInfo(
notFoundRoute,
notFoundRoute,
@@ -2338,10 +2332,10 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
}
);
- case 106:
+ case 105:
routeInfo = _context.sent;
- case 107:
+ case 106:
Router.events.emit(
"beforeHistoryChange",
as,
@@ -2382,7 +2376,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
y: 0
}
: null;
- _context.next = 116;
+ _context.next = 115;
return this.set(
route,
pathname,
@@ -2395,9 +2389,9 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
else throw e;
});
- case 116:
+ case 115:
if (!error) {
- _context.next = 119;
+ _context.next = 118;
break;
}
@@ -2409,7 +2403,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
);
throw error;
- case 119:
+ case 118:
if (false) {
}
@@ -2420,21 +2414,21 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
);
return _context.abrupt("return", true);
- case 124:
- _context.prev = 124;
+ case 123:
+ _context.prev = 123;
_context.t2 = _context["catch"](77);
if (!_context.t2.cancelled) {
- _context.next = 128;
+ _context.next = 127;
break;
}
return _context.abrupt("return", false);
- case 128:
+ case 127:
throw _context.t2;
- case 129:
+ case 128:
case "end":
return _context.stop();
}
@@ -2444,8 +2438,8 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
this,
[
[36, 46],
- [77, 124],
- [95, 101]
+ [77, 123],
+ [94, 100]
]
);
})
Diff for index.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.20a22da5dd60e15fb078.js"
+ src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.2f224f5f90b8eee0d83e.js"
defer=""
></script>
<script
Diff for link.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.20a22da5dd60e15fb078.js"
+ src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.2f224f5f90b8eee0d83e.js"
defer=""
></script>
<script
Diff for withRouter.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.20a22da5dd60e15fb078.js"
+ src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.2f224f5f90b8eee0d83e.js"
defer=""
></script>
<script
…ercel#25990) This PR is an attempt to fix vercel#25943. The implementation is only a proposal and I'll be happy to take any feedback about it :) Fixes: vercel#25943 ## Bug - [X] Related issues linked using `fixes #number` - [x] Integration tests added ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. ## Documentation / Examples - [ ] Make sure the linting passes
This PR is an attempt to fix #25943. The implementation is only a proposal and I'll be happy to take any feedback about it :)
Fixes: #25943
Bug
fixes #number
Feature
fixes #number
Documentation / Examples