[middleware] Fix hydration for rewrites to dynamic pages #32534

javivelasco · 2021-12-15T11:53:30Z

This PR fixes an issue on hydration where we rewrote using a middleware to a dynamic path. In such cases we need to run preflight to find out the middleware operation metadata.

Bug

Related issues linked using fixes #number
Integration tests added
Errors have helpful link attached, see contributing.md

ijjk · 2021-12-15T12:15:38Z

Failing test suites

Commit: b4b11d0

test/integration/middleware/core/test/index.test.js

Middleware base tests > dev mode > should rewrite without hard navigation
Middleware base tests > dev mode > /fr should rewrite without hard navigation
Middleware base tests > production mode > should rewrite without hard navigation
Middleware base tests > production mode > /fr should rewrite without hard navigation

Expand output

● Middleware base tests › dev mode › should rewrite without hard navigation

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: undefined

  140 |       context.appPort,
  141 |       `${locale}/rewrites/rewrite-me-to-about`
> 142 |     )
      |      ^
  143 |     const html = await res.text()
  144 |     const $ = cheerio.load(html)
  145 |     const browser = await webdriver(

  at Object.<anonymous> (integration/middleware/core/test/index.test.js:142:58)

● Middleware base tests › dev mode › /fr should rewrite without hard navigation

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: undefined

  140 |       context.appPort,
  141 |       `${locale}/rewrites/rewrite-me-to-about`
> 142 |     )
      |      ^
  143 |     const html = await res.text()
  144 |     const $ = cheerio.load(html)
  145 |     const browser = await webdriver(

  at Object.<anonymous> (integration/middleware/core/test/index.test.js:142:58)

● Middleware base tests › production mode › should rewrite without hard navigation

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: undefined

  140 |       context.appPort,
  141 |       `${locale}/rewrites/rewrite-me-to-about`
> 142 |     )
      |      ^
  143 |     const html = await res.text()
  144 |     const $ = cheerio.load(html)
  145 |     const browser = await webdriver(

  at Object.<anonymous> (integration/middleware/core/test/index.test.js:142:58)

● Middleware base tests › production mode › /fr should rewrite without hard navigation

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: undefined

  140 |       context.appPort,
  141 |       `${locale}/rewrites/rewrite-me-to-about`
> 142 |     )
      |      ^
  143 |     const html = await res.text()
  144 |     const $ = cheerio.load(html)
  145 |     const browser = await webdriver(

  at Object.<anonymous> (integration/middleware/core/test/index.test.js:142:58)

ijjk · 2021-12-15T13:07:24Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	javivelasco/next.js fix-rewrite-bug	Change
buildDuration	22.5s	22.1s	-495ms
buildDurationCached	4.2s	4.1s	-79ms
nodeModulesSize	350 MB	350 MB	⚠️ +183 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	javivelasco/next.js fix-rewrite-bug	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.579	3.467	-0.11
/ avg req/sec	698.61	721.15	+22.54
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.673	1.645	-0.03
/error-in-render avg req/sec	1494.6	1519.93	+25.33

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

	vercel/next.js canary	javivelasco/next.js fix-rewrite-bug	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.2 kB	42.2 kB	✓
main-HASH.js gzip	30.2 kB	30.2 kB	⚠️ +46 B
webpack-HASH.js gzip	1.45 kB	1.45 kB	✓
Overall change	74 kB	74 kB	⚠️ +46 B

Legacy Client Bundles (polyfills)

	vercel/next.js canary	javivelasco/next.js fix-rewrite-bug	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	javivelasco/next.js fix-rewrite-bug	Change
_app-HASH.js gzip	1.37 kB	1.37 kB	✓
_error-HASH.js gzip	194 B	194 B	✓
amp-HASH.js gzip	312 B	312 B	✓
css-HASH.js gzip	326 B	326 B	✓
dynamic-HASH.js gzip	2.37 kB	2.37 kB	✓
head-HASH.js gzip	350 B	350 B	✓
hooks-HASH.js gzip	919 B	919 B	✓
image-HASH.js gzip	4.73 kB	4.73 kB	✓
index-HASH.js gzip	263 B	263 B	✓
link-HASH.js gzip	2.13 kB	2.13 kB	✓
routerDirect..HASH.js gzip	321 B	321 B	✓
script-HASH.js gzip	383 B	383 B	✓
withRouter-HASH.js gzip	318 B	318 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	14.1 kB	14.1 kB	✓

Client Build Manifests

	vercel/next.js canary	javivelasco/next.js fix-rewrite-bug	Change
_buildManifest.js gzip	459 B	459 B	✓
Overall change	459 B	459 B	✓

Rendered Page Sizes Overall decrease ✓

	vercel/next.js canary	javivelasco/next.js fix-rewrite-bug	Change
index.html gzip	533 B	532 B	-1 B
link.html gzip	547 B	545 B	-2 B
withRouter.html gzip	527 B	527 B	✓
Overall change	1.61 kB	1.6 kB	-3 B

Diffs

Diff for main-HASH.js

@@ -5032,7 +5032,17 @@
                                 delBasePath(resolvedAs),
                                 this.locale
                               );
-                              if (!(options._h !== 1)) {
+                              if (
+                                !(
+                                  options._h !== 1 ||
+                                  (0, _isDynamic).isDynamicRoute(
+                                    (0,
+                                    _normalizeTrailingSlash).removePathTrailingSlash(
+                                      pathname
+                                    )
+                                  )
+                                )
+                              ) {
                                 _ctx.next = 84;
                                 break;
                               }
@@ -5095,7 +5105,12 @@
                                 new Promise(function() {})
                               );
                             case 81:
-                              if (!(effect.type === "refresh")) {
+                              if (
+                                !(
+                                  effect.type === "refresh" &&
+                                  as !== window.location.pathname
+                                )
+                              ) {
                                 _ctx.next = 84;
                                 break;
                               }

Diff for index.html

@@ -19,7 +19,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-2115eccd80a7ec6c.js"
+      src="/_next/static/chunks/main-dde7df6b4cad59bb.js"
       defer=""
     ></script>
     <script

Diff for link.html

@@ -19,7 +19,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-2115eccd80a7ec6c.js"
+      src="/_next/static/chunks/main-dde7df6b4cad59bb.js"
       defer=""
     ></script>
     <script

Diff for withRouter.html

@@ -19,7 +19,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-2115eccd80a7ec6c.js"
+      src="/_next/static/chunks/main-dde7df6b4cad59bb.js"
       defer=""
     ></script>
     <script

Default Build with SWC (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	javivelasco/next.js fix-rewrite-bug	Change
buildDuration	22.7s	23.4s	⚠️ +696ms
buildDurationCached	3.8s	4.1s	⚠️ +252ms
nodeModulesSize	350 MB	350 MB	⚠️ +183 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	javivelasco/next.js fix-rewrite-bug	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.386	3.443	⚠️ +0.06
/ avg req/sec	738.23	726.04	⚠️ -12.19
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.547	1.626	⚠️ +0.08
/error-in-render avg req/sec	1616	1537.4	⚠️ -78.6

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

	vercel/next.js canary	javivelasco/next.js fix-rewrite-bug	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.3 kB	42.3 kB	✓
main-HASH.js gzip	30.3 kB	30.3 kB	⚠️ +12 B
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	74.2 kB	74.3 kB	⚠️ +12 B

Legacy Client Bundles (polyfills)

	vercel/next.js canary	javivelasco/next.js fix-rewrite-bug	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	javivelasco/next.js fix-rewrite-bug	Change
_app-HASH.js gzip	1.35 kB	1.35 kB	✓
_error-HASH.js gzip	180 B	180 B	✓
amp-HASH.js gzip	305 B	305 B	✓
css-HASH.js gzip	321 B	321 B	✓
dynamic-HASH.js gzip	2.36 kB	2.36 kB	✓
head-HASH.js gzip	342 B	342 B	✓
hooks-HASH.js gzip	906 B	906 B	✓
image-HASH.js gzip	4.75 kB	4.75 kB	✓
index-HASH.js gzip	256 B	256 B	✓
link-HASH.js gzip	2.19 kB	2.19 kB	✓
routerDirect..HASH.js gzip	314 B	314 B	✓
script-HASH.js gzip	375 B	375 B	✓
withRouter-HASH.js gzip	309 B	309 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	14.1 kB	14.1 kB	✓

Client Build Manifests

	vercel/next.js canary	javivelasco/next.js fix-rewrite-bug	Change
_buildManifest.js gzip	459 B	459 B	✓
Overall change	459 B	459 B	✓

Rendered Page Sizes Overall increase ⚠️

	vercel/next.js canary	javivelasco/next.js fix-rewrite-bug	Change
index.html gzip	531 B	531 B	✓
link.html gzip	544 B	545 B	⚠️ +1 B
withRouter.html gzip	524 B	526 B	⚠️ +2 B
Overall change	1.6 kB	1.6 kB	⚠️ +3 B

Diffs

Diff for main-HASH.js

@@ -5032,7 +5032,17 @@
                                 delBasePath(resolvedAs),
                                 this.locale
                               );
-                              if (!(options._h !== 1)) {
+                              if (
+                                !(
+                                  options._h !== 1 ||
+                                  (0, _isDynamic).isDynamicRoute(
+                                    (0,
+                                    _normalizeTrailingSlash).removePathTrailingSlash(
+                                      pathname
+                                    )
+                                  )
+                                )
+                              ) {
                                 _ctx.next = 84;
                                 break;
                               }
@@ -5095,7 +5105,12 @@
                                 new Promise(function() {})
                               );
                             case 81:
-                              if (!(effect.type === "refresh")) {
+                              if (
+                                !(
+                                  effect.type === "refresh" &&
+                                  as !== window.location.pathname
+                                )
+                              ) {
                                 _ctx.next = 84;
                                 break;
                               }

Diff for index.html

@@ -19,7 +19,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-2115eccd80a7ec6c.js"
+      src="/_next/static/chunks/main-dde7df6b4cad59bb.js"
       defer=""
     ></script>
     <script

Diff for link.html

@@ -19,7 +19,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-2115eccd80a7ec6c.js"
+      src="/_next/static/chunks/main-dde7df6b4cad59bb.js"
       defer=""
     ></script>
     <script

Diff for withRouter.html

@@ -19,7 +19,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-2115eccd80a7ec6c.js"
+      src="/_next/static/chunks/main-dde7df6b4cad59bb.js"
       defer=""
     ></script>
     <script

Commit: b46f41a

Add failing test

f672aa6

javivelasco requested review from huozhi, ijjk, shuding and timneutkens as code owners December 15, 2021 11:53

ijjk added the type: next label Dec 15, 2021

This comment has been minimized.

Sign in to view

Bug fix

b46f41a

javivelasco force-pushed the fix-rewrite-bug branch from b4b11d0 to b46f41a Compare December 15, 2021 12:48

timneutkens approved these changes Dec 15, 2021

View reviewed changes

kodiakhq bot merged commit 3da1f17 into vercel:canary Dec 15, 2021

albertpurnama mentioned this pull request Jan 10, 2022

"Unhandled Runtime Error, incompatible-href-as" when using _middleware.ts rewrites with getStaticProps + fallback true as well as when using CSR only. #32446

Closed

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

javivelasco deleted the fix-rewrite-bug branch April 26, 2022 20:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[middleware] Fix hydration for rewrites to dynamic pages #32534

[middleware] Fix hydration for rewrites to dynamic pages #32534

javivelasco commented Dec 15, 2021

This comment has been minimized.

ijjk commented Dec 15, 2021

ijjk commented Dec 15, 2021

Diffs

Diffs

[middleware] Fix hydration for rewrites to dynamic pages #32534

[middleware] Fix hydration for rewrites to dynamic pages #32534

Conversation

javivelasco commented Dec 15, 2021

Bug

This comment has been minimized.

ijjk commented Dec 15, 2021

Failing test suites

ijjk commented Dec 15, 2021

Stats from current PR

Diffs

Diffs