Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

"scroll" is sometimes undefined in scrollTo(scroll.x, scroll.y) #1592

Closed
AnandChowdhary opened this issue Oct 8, 2020 · 9 comments · Fixed by #1594
Closed

"scroll" is sometimes undefined in scrollTo(scroll.x, scroll.y) #1592

AnandChowdhary opened this issue Oct 8, 2020 · 9 comments · Fixed by #1594

Comments

@AnandChowdhary
Copy link
Contributor

Describe the bug
I got this error on our production website, tracked using Sentry:

TypeError: Cannot read property 'x' of undefined
  at ? (/client/client.8caa5d97.js:16:2552)
  at Generator.next(<anonymous>)
  at a(/client/client.8caa5d97.js:16:86)

Here's the link: https://sentry.io/share/issue/20ae1300a33f48f58457ffce7c96925a/

Fix
I was able to zero down on the line here: https://github.com/sveltejs/sapper/blob/master/runtime/src/app/router/index.ts#L223. It seems like sometimes let scroll = scroll_history[id]; might be undefined, and then calling scrollTo(scroll.x, scroll.y) throws an error. A proposed fix for this is to change:

if (popstate || deep_linked) {
  scrollTo(scroll.x, scroll.y);
}

to:

if (scroll && (popstate || deep_linked)) {
  scrollTo(scroll.x, scroll.y);
}

Stacktraces

Screen Shot 2020-10-08 at 13 30 45

Information about your Sapper Installation:

  • The output of npx envinfo --system --npmPackages svelte,sapper,rollup,webpack --binaries --browsers
Expand
System:
    OS: macOS 10.15.6
    CPU: (8) x64 Intel(R) Core(TM) i5-1038NG7 CPU @ 2.00GHz
    Memory: 1002.30 MB / 16.00 GB
    Shell: 5.7.1 - /bin/zsh
  Binaries:
    Node: 14.5.0 - /usr/local/bin/node
    npm: 6.14.6 - ~/Projects/koj/koj/node_modules/.bin/npm
  Browsers:
    Chrome: 85.0.4183.121
    Safari: 14.0
  npmPackages:
    rollup: ^2.29.0 => 2.29.0 
    sapper: ^0.28.10 => 0.28.10 
  • Browser details from Sentry:

Screen Shot 2020-10-08 at 13 31 37

  • Your hosting environment: AWS EC2 running a Docker container that serves the full-stack Sapper server

  • If it is an exported (npm run export) or dynamic application: Dynamic

@AnandChowdhary
Copy link
Contributor Author

@Conduitry Is this an invalid issue?

@claudijo
Copy link

claudijo commented Oct 9, 2020

I also wonder why this was closed @Conduitry. I experience the same intermittent issue with "Cannot read property 'x' of undefined" as described above using latest Sapper (v0.28.10)

@AnandChowdhary
Copy link
Contributor Author

I've opened a PR to make sure that scroll exists before trying to access its PR.

@AnandChowdhary
Copy link
Contributor Author

@Conduitry, do you think we can open this issue? I've seen this error 14 times on production in the past two weeks.

@rcabralc
Copy link

I can reproduce it:

  • start a new Sapper project from the Sapper template, get it running, as usual
  • click on "blog" - this creates an entry in the scroll history
  • click on "What is Sapper?" - this creates another entry
  • click on "Svelte" - it's important to be a link to outside the application, this will wipe out scroll history
  • go back - works without error
  • go back again - works, but the error happens

Sapper stores/recovers only an id from the History API, not the scroll state. The first time we go back from the Svelte site to the application the browser takes care of the scroll, so no error happens. The next time, Sapper tries to restore the scroll position, but it can't because scroll data was gone in the moment we went outside the application (clicking the Svelte link).

@AnandChowdhary
Copy link
Contributor Author

Looks like @rcabralc has reproduced this well. I'm seeing this too. Let's re-open this issue and evaluate my PR #1594? I think just checking for scroll existing before accessing it is enough.

🔔 Can we re-open this issue? @benmccann @Conduitry

@benmccann benmccann reopened this Oct 22, 2020
@zolotokrylin
Copy link

We also have this issue happening from time to time.

TypeError: undefined is not an object (evaluating 'n.x')
  File "webpack:///./src/node_modules/@sapper/app.mjs", line 225, in navigate
                    scrollTo(scroll.x, scroll.y);
  File "[native code]", line unknown, in generatorResume
  File "webpack:///./src/node_modules/@sapper/app.mjs", line 25, in __awaiter
            function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
  File "[native code]", line unknown, in promiseReactionJob

@benmccann
Copy link
Member

I re-implemented scroll tracking in SvelteKit so that it's both simpler and will work if you leave the site and come back

I merged #1594 to avoid the issue in Sapper. Scroll history will be lost if you leave the site and come back, but the errors will no longer occur

@zolotokrylin
Copy link

When it is planned to release the fixes listed in "Unreleased" section?

BlakeMScurr added a commit to BlakeMScurr/RealKeys that referenced this issue Mar 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants