Incorrect scrolling behavior when opening links with a hash

lovasoa commented 3 years ago

Describe the bug

Hash links do not always scroll to the right position in the target page.

Reproduction

Here is a repo that reproduces the bug: https://github.com/lovasoa/sanipasse

From the root of the repo, run npm run dev, then on http://localhost:3000 click the first link ("strictement privée"). You'll end up at the bottom of the page on http://localhost:3000/apropos#donnees instead of scrolling to the right heading ("🔒 Protection des données personnelles"). You can see that svelte's scrolling behavior is incorrect by comparing the same link when opened in a new tab.

Logs

No response

System Info

System:
    OS: Linux 5.13 Fedora 34 (Workstation Edition) 34 (Workstation Edition)
    CPU: (8) x64 Intel(R) Core(TM) i7-10510U CPU @ 1.80GHz
    Memory: 3.66 GB / 15.28 GB
    Container: Yes
    Shell: 5.1.0 - /bin/bash
  Binaries:
    Node: 16.4.0 - /usr/bin/node
    Yarn: 1.22.10 - /usr/bin/yarn
    npm: 7.18.1 - /usr/bin/npm
  Browsers:
    Firefox: 90.0.2
  npmPackages:
    @sveltejs/adapter-node: next => 1.0.0-next.35 
    @sveltejs/adapter-static: next => 1.0.0-next.13 
    @sveltejs/kit: next => 1.0.0-next.137 
    svelte: ^3.38.3 => 3.41.0

Severity

serious, but I can work around it

Additional Information

By adding a breakpoint on

            deep_linked.scrollIntoView();

in svelte's router.js you see where the problem is. The call to scrollIntoView happens before the contents from the old page are removed from the DOM.

benmccann commented 3 years ago

I was not able to run your example. After doing npm install and then npm run dev, I get:

performance is not defined
ReferenceError: performance is not defined

lovasoa commented 3 years ago

The code requires node >=16. Are you on the latest version of node ?

lovasoa commented 3 years ago

There is also a Dockerfile provided if you don't want to install anything locally.

benmccann commented 3 years ago

Ok, I switched to Node 16 and it runs now, but I can't reproduce it. Could it possible be a race condition? Can you explain more about what you believe the cause of the issue is?

lovasoa commented 3 years ago

To help with reproduction, here is my npm ls :

sanipasse@1.0 /workspaces/sanipasse
├── @ctrl/ts-base32@1.2.6
├── @peculiar/x509@1.3.2
├── @sveltejs/adapter-node@1.0.0-next.33
├── @sveltejs/adapter-static@1.0.0-next.13
├── @sveltejs/kit@1.0.0-next.127
├── @types/pako@1.0.2
├── @typescript-eslint/eslint-plugin@4.28.3
├── @typescript-eslint/parser@4.28.3
├── @zxing/browser@0.0.9
├── @zxing/library@0.18.6
├── ajv@8.6.1
├── base45-ts@1.0.3
├── base64-js@1.5.1
├── bignumber.js@9.0.1
├── bootstrap-icons@1.5.0
├── bootstrap@5.0.2
├── buffer@6.0.3
├── cbor-web@7.0.6
├── cbor@7.0.6
├── cosette@0.6.1
├── eslint-config-prettier@8.3.0
├── eslint-plugin-svelte3@3.2.0
├── eslint@7.30.0
├── isomorphic-webcrypto@2.3.8
├── localforage@1.9.0
├── node-fetch@2.6.1
├── pako@2.0.3
├── pdfjs-dist@2.8.335
├── prettier-plugin-svelte@2.3.1
├── prettier@2.2.1
├── sequelize@6.6.5
├── sqlite3@5.0.2
├── svelte-check@1.6.0
├── svelte-preprocess@4.7.4
├── svelte@3.38.3
├── sveltestrap@5.4.0
├── tslib@2.3.0
├── typescript@4.3.5
├── workbox-precaching@6.1.5
└── workbox-recipes@6.1.5

When placing a breakpoint on deep_linked.scrollIntoView(); in router.js. I can see that it is being called while the contents of the old page (the one on which the |<a> tag is) is still in the dom and and displayed on screen when it is called.

lovasoa commented 3 years ago

@benmccann : my website https://sanipasse.fr/ uses multiple internal hash links and suffers from this bug. What are the next steps towards fixing it ? Do you need any further information from me ?

dummdidumm commented 2 years ago

No longer reproducible. The code in question now has await tick() before it which should give enough time to rerender the components so scrolling works.

sveltejs / kit