Fix next/link can't jump to non-latin anchors #36430

hulufei · 2022-04-25T06:03:29Z

fixes #11109

Bug

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

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.
Errors have helpful link attached, see contributing.md

Documentation / Examples

Make sure the linting passes by running yarn lint

hulufei · 2022-04-25T06:08:37Z

I have made it works for anchors in the same page, but got stuck with links from another page. Could anyone help me to work it out? Thanks.

hulufei · 2022-04-25T09:56:50Z

I think I figured it out.

ijjk · 2022-04-25T12:56:52Z

Failing test suites

Commit: 2900d7e

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

Middleware base tests > production mode > should override with rewrite externally correctly
Middleware base tests > production mode > should rewrite to Vercel

Expand output

● Middleware base tests › production mode › should override with rewrite externally correctly

elementHandle.click: Timeout 30000ms exceeded.
=========================== logs ===========================
attempting click action
  waiting for element to be visible, enabled and stable
  element is visible, enabled and stable
  scrolling into view if needed
  done scrolling
  checking that element receives pointer events at (94.19,148.47)
  element does receive pointer events
  performing click action
  click action done
  waiting for scheduled navigations to finish
============================================================

  281 |   click() {
  282 |     return this.chain((el) => {
> 283 |       return el.click().then(() => el)
      |                 ^
  284 |     })
  285 |   }
  286 |

  at lib/browsers/playwright.ts:283:17
  at Object.<anonymous> (integration/middleware/core/test/index.test.js:266:5)

● Middleware base tests › production mode › should rewrite to Vercel

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

Expected: "Develop. Preview. Ship. For the best frontend teams – Vercel"
Received: ""

  478 |     // const browser = await webdriver(context.appPort, '/rewrite-me-to-vercel')
  479 |     // TODO: running this to chech the window.location.pathname hangs for some reason;
> 480 |     expect($('head > title').text()).toBe(
      |                                      ^
  481 |       'Develop. Preview. Ship. For the best frontend teams – Vercel'
  482 |     )
  483 |   })

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

Read more about building and testing Next.js in contributing.md.

ijjk · 2022-04-25T13:08:22Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	hulufei/next.js fix-11109	Change
buildDuration	19.3s	19.4s	⚠️ +148ms
buildDurationCached	7.6s	7.6s	⚠️ +15ms
nodeModulesSize	479 MB	479 MB	⚠️ +593 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	hulufei/next.js fix-11109	Change
/ failed reqs	0	0	✓
/ total time (seconds)	5.172	5.107	-0.06
/ avg req/sec	483.34	489.48	+6.14
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	2	2.01	⚠️ +0.01
/error-in-render avg req/sec	1249.76	1243.78	⚠️ -5.98

Client Bundles (main, webpack) Overall increase ⚠️

	vercel/next.js canary	hulufei/next.js fix-11109	Change
925.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42 kB	42 kB	✓
main-HASH.js gzip	29 kB	29 kB	⚠️ +32 B
webpack-HASH.js gzip	1.54 kB	1.54 kB	✓
Overall change	72.7 kB	72.8 kB	⚠️ +32 B

Legacy Client Bundles (polyfills)

	vercel/next.js canary	hulufei/next.js fix-11109	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	hulufei/next.js fix-11109	Change
_app-HASH.js gzip	1.36 kB	1.36 kB	✓
_error-HASH.js gzip	193 B	193 B	✓
amp-HASH.js gzip	308 B	308 B	✓
css-HASH.js gzip	327 B	327 B	✓
dynamic-HASH.js gzip	2.71 kB	2.71 kB	✓
head-HASH.js gzip	359 B	359 B	✓
hooks-HASH.js gzip	920 B	920 B	✓
image-HASH.js gzip	5.73 kB	5.73 kB	✓
index-HASH.js gzip	263 B	263 B	✓
link-HASH.js gzip	2.65 kB	2.65 kB	✓
routerDirect..HASH.js gzip	320 B	320 B	✓
script-HASH.js gzip	391 B	391 B	✓
withRouter-HASH.js gzip	318 B	318 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	16 kB	16 kB	✓

Client Build Manifests

	vercel/next.js canary	hulufei/next.js fix-11109	Change
_buildManifest.js gzip	458 B	458 B	✓
Overall change	458 B	458 B	✓

Rendered Page Sizes Overall decrease ✓

	vercel/next.js canary	hulufei/next.js fix-11109	Change
index.html gzip	533 B	532 B	-1 B
link.html gzip	547 B	546 B	-1 B
withRouter.html gzip	528 B	527 B	-1 B
Overall change	1.61 kB	1.6 kB	-3 B

Diffs

Diff for main-HASH.js

@@ -5220,7 +5220,8 @@
                     isValidShallowRoute,
                     _scroll1,
                     shouldScroll,
-                    resetScroll;
+                    resetScroll,
+                    hashRegex;
                   return _runtimeJs.default.wrap(
                     function _callee$(_ctx) {
                       while (1)
@@ -5871,9 +5872,13 @@
                               as,
                               routeProps
                             );
+                            hashRegex = /#.+$/;
+                            if (shouldScroll && hashRegex.test(as)) {
+                              _this.scrollToHash(as);
+                            }
                             return _ctx.abrupt("return", true);
-                          case 164:
-                            _ctx.prev = 164;
+                          case 166:
+                            _ctx.prev = 166;
                             _ctx.t4 = _ctx["catch"](113);
                             if (
                               !(
@@ -5881,13 +5886,13 @@
                                 _ctx.t4.cancelled
                               )
                             ) {
-                              _ctx.next = 168;
+                              _ctx.next = 170;
                               break;
                             }
                             return _ctx.abrupt("return", false);
-                          case 168:
+                          case 170:
                             throw _ctx.t4;
-                          case 169:
+                          case 171:
                           case "end":
                             return _ctx.stop();
                         }
@@ -5896,7 +5901,7 @@
                     null,
                     [
                       [39, 51],
-                      [113, 164],
+                      [113, 166],
                       [135, 141]
                     ]
                   );
@@ -6326,15 +6331,17 @@
                 window.scrollTo(0, 0);
                 return;
               }
+              // Decode hash to make non-latin anchor works.
+              var rawHash = decodeURIComponent(hash);
               // First we check if the element by id is found
-              var idEl = document.getElementById(hash);
+              var idEl = document.getElementById(rawHash);
               if (idEl) {
                 idEl.scrollIntoView();
                 return;
               }
               // If there's no element with the id, we check the `name` property
               // To mirror browsers
-              var nameEl = document.getElementsByName(hash)[0];
+              var nameEl = document.getElementsByName(rawHash)[0];
               if (nameEl) {
                 nameEl.scrollIntoView();
               }

Diff for index.html

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

Diff for link.html

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

Diff for withRouter.html

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

Default Build with SWC (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	hulufei/next.js fix-11109	Change
buildDuration	23.3s	23.1s	-205ms
buildDurationCached	7.9s	8s	⚠️ +119ms
nodeModulesSize	479 MB	479 MB	⚠️ +593 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	hulufei/next.js fix-11109	Change
/ failed reqs	0	0	✓
/ total time (seconds)	5.418	5.399	-0.02
/ avg req/sec	461.39	463.04	+1.65
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	2.225	2.137	-0.09
/error-in-render avg req/sec	1123.69	1169.92	+46.23

Client Bundles (main, webpack) Overall increase ⚠️

	vercel/next.js canary	hulufei/next.js fix-11109	Change
925.HASH.js gzip	178 B	178 B	✓
framework-HASH.js gzip	42.7 kB	42.7 kB	✓
main-HASH.js gzip	29.5 kB	29.5 kB	⚠️ +34 B
webpack-HASH.js gzip	1.54 kB	1.54 kB	✓
Overall change	73.9 kB	73.9 kB	⚠️ +34 B

Legacy Client Bundles (polyfills)

	vercel/next.js canary	hulufei/next.js fix-11109	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	hulufei/next.js fix-11109	Change
_app-HASH.js gzip	1.35 kB	1.35 kB	✓
_error-HASH.js gzip	179 B	179 B	✓
amp-HASH.js gzip	311 B	311 B	✓
css-HASH.js gzip	324 B	324 B	✓
dynamic-HASH.js gzip	2.89 kB	2.89 kB	✓
head-HASH.js gzip	357 B	357 B	✓
hooks-HASH.js gzip	920 B	920 B	✓
image-HASH.js gzip	5.82 kB	5.82 kB	✓
index-HASH.js gzip	261 B	261 B	✓
link-HASH.js gzip	2.78 kB	2.78 kB	✓
routerDirect..HASH.js gzip	322 B	322 B	✓
script-HASH.js gzip	392 B	392 B	✓
withRouter-HASH.js gzip	317 B	317 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	16.3 kB	16.3 kB	✓

Client Build Manifests

	vercel/next.js canary	hulufei/next.js fix-11109	Change
_buildManifest.js gzip	457 B	457 B	✓
Overall change	457 B	457 B	✓

Rendered Page Sizes Overall increase ⚠️

	vercel/next.js canary	hulufei/next.js fix-11109	Change
index.html gzip	532 B	533 B	⚠️ +1 B
link.html gzip	546 B	547 B	⚠️ +1 B
withRouter.html gzip	528 B	529 B	⚠️ +1 B
Overall change	1.61 kB	1.61 kB	⚠️ +3 B

Diffs

Diff for main-HASH.js

@@ -5220,7 +5220,8 @@
                     isValidShallowRoute,
                     _scroll1,
                     shouldScroll,
-                    resetScroll;
+                    resetScroll,
+                    hashRegex;
                   return _runtimeJs.default.wrap(
                     function _callee$(_ctx) {
                       while (1)
@@ -5871,9 +5872,13 @@
                               as,
                               routeProps
                             );
+                            hashRegex = /#.+$/;
+                            if (shouldScroll && hashRegex.test(as)) {
+                              _this.scrollToHash(as);
+                            }
                             return _ctx.abrupt("return", true);
-                          case 164:
-                            _ctx.prev = 164;
+                          case 166:
+                            _ctx.prev = 166;
                             _ctx.t4 = _ctx["catch"](113);
                             if (
                               !(
@@ -5881,13 +5886,13 @@
                                 _ctx.t4.cancelled
                               )
                             ) {
-                              _ctx.next = 168;
+                              _ctx.next = 170;
                               break;
                             }
                             return _ctx.abrupt("return", false);
-                          case 168:
+                          case 170:
                             throw _ctx.t4;
-                          case 169:
+                          case 171:
                           case "end":
                             return _ctx.stop();
                         }
@@ -5896,7 +5901,7 @@
                     null,
                     [
                       [39, 51],
-                      [113, 164],
+                      [113, 166],
                       [135, 141]
                     ]
                   );
@@ -6326,15 +6331,17 @@
                 window.scrollTo(0, 0);
                 return;
               }
+              // Decode hash to make non-latin anchor works.
+              var rawHash = decodeURIComponent(hash);
               // First we check if the element by id is found
-              var idEl = document.getElementById(hash);
+              var idEl = document.getElementById(rawHash);
               if (idEl) {
                 idEl.scrollIntoView();
                 return;
               }
               // If there's no element with the id, we check the `name` property
               // To mirror browsers
-              var nameEl = document.getElementsByName(hash)[0];
+              var nameEl = document.getElementsByName(rawHash)[0];
               if (nameEl) {
                 nameEl.scrollIntoView();
               }

Diff for index.html

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

Diff for link.html

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

Diff for withRouter.html

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

Commit: 8e0787c

hulufei · 2022-04-27T11:31:41Z

@ijjk Could you check it again? Thanks.

hulufei · 2022-04-28T02:08:50Z

It's weird, I checked the failing log, and didn't find any assert failures. I had run yarn testheadless --testPathPattern "client-navigation" -t 'Client Navigation' on local with 2900d7e, all tests passed.

Anything I can do to continue?

packages/next/shared/lib/router/router.ts

ijjk · 2022-05-22T06:05:13Z

Thanks for the PR!

ijjk added the type: next label Apr 25, 2022

hulufei changed the title ~~Fix next/link can't jump to non-latin anchors~~ [Help needed] Fix next/link can't jump to non-latin anchors Apr 25, 2022

hulufei changed the title ~~[Help needed] Fix next/link can't jump to non-latin anchors~~ [Need help] Fix next/link can't jump to non-latin anchors Apr 25, 2022

hulufei changed the title ~~[Need help] Fix next/link can't jump to non-latin anchors~~ Fix next/link can't jump to non-latin anchors Apr 25, 2022

hulufei marked this pull request as ready for review April 25, 2022 09:57

hulufei requested review from timneutkens, ijjk, shuding and huozhi as code owners April 25, 2022 09:57

hulufei added 3 commits April 26, 2022 13:14

Fix next/link can't jump to non-latin anchors

3fadb41

Fix jump to another page by non-latin anchors

a87e75c

Fix regression

cfc2dea

hulufei force-pushed the fix-11109 branch from bb86672 to cfc2dea Compare April 26, 2022 05:14

Merge branch 'canary' into fix-11109

2900d7e

ijjk reviewed Apr 28, 2022

View reviewed changes

packages/next/shared/lib/router/router.ts Outdated Show resolved Hide resolved

Move hash decoding directly to scrollToHash

665971c

hulufei requested a review from ijjk May 2, 2022 04:38

ijjk reviewed May 4, 2022

View reviewed changes

packages/next/shared/lib/router/router.ts Show resolved Hide resolved

Merge branch 'canary' into fix-11109

8e0787c

ijjk approved these changes May 22, 2022

View reviewed changes

kodiakhq bot merged commit 11cb49b into vercel:canary May 22, 2022

dleard mentioned this pull request Jun 17, 2022

Investigate if the anchor useEffect is still required in the tasklist bcgov/cas-cif#751

Closed

github-actions bot locked as resolved and limited conversation to collaborators Jun 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix next/link can't jump to non-latin anchors #36430

Fix next/link can't jump to non-latin anchors #36430

hulufei commented Apr 25, 2022

hulufei commented Apr 25, 2022 •

edited

Loading

hulufei commented Apr 25, 2022 •

edited

Loading

ijjk commented Apr 25, 2022 •

edited

Loading

ijjk commented Apr 25, 2022 •

edited

Loading

Diffs

Diffs

hulufei commented Apr 27, 2022

hulufei commented Apr 28, 2022

ijjk commented May 22, 2022

Fix next/link can't jump to non-latin anchors #36430

Fix next/link can't jump to non-latin anchors #36430

Conversation

hulufei commented Apr 25, 2022

Bug

Feature

Documentation / Examples

hulufei commented Apr 25, 2022 • edited Loading

hulufei commented Apr 25, 2022 • edited Loading

ijjk commented Apr 25, 2022 • edited Loading

Failing test suites

ijjk commented Apr 25, 2022 • edited Loading

Stats from current PR

Diffs

Diffs

hulufei commented Apr 27, 2022

hulufei commented Apr 28, 2022

ijjk commented May 22, 2022

hulufei commented Apr 25, 2022 •

edited

Loading

hulufei commented Apr 25, 2022 •

edited

Loading

ijjk commented Apr 25, 2022 •

edited

Loading

ijjk commented Apr 25, 2022 •

edited

Loading