Skip to content
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

useWindowScroll may lose window scroll change at mount #1699

Closed
eaglus opened this issue Dec 11, 2020 · 0 comments
Closed

useWindowScroll may lose window scroll change at mount #1699

eaglus opened this issue Dec 11, 2020 · 0 comments

Comments

@eaglus
Copy link
Contributor

eaglus commented Dec 11, 2020

What is the current behavior?
Now, useWindowScroll asks for window scroll position at two points:

But, react render may be async, so, between first render and execution of useEffect may happen a "time gap", and window scroll position may be updated there (in that "gap").
For example, some other effects (of other components may cause window scroll).
And that updated value of window scroll will be lost, becase window.addEventListener is called before that possible update.
We need to update window scroll inside of effect handler, before subscription.
Also, we can use lazy initial value for state (to prevent scroll position recalculation at each render).
It is legal, see there: https://kentcdodds.com/blog/use-state-lazy-initialization-and-function-updates

Steps to reproduce it and if possible a minimal demo of the problem
This is very unstable bug, it depends on heighbor/parent/child configuration, and i can reproduce it on my application (when some route gets mounted, and window scrolled to top, useWindowScroll gets remounted, but, its new instance has wrong scroll position value).

Uploading image.png…

What is the expected behavior?
Always have correct hook result, correspondent to window scroll position

A little about versions:

  • OS: any
  • Browser (vendor and version): any
  • React: 17.01
  • react-use: 15.3.4
  • Did this worked in the previous package version?: no
@eaglus eaglus changed the title useWindowScroll may loose window scroll change at first mount useWindowScroll may lose window scroll change at first mount Dec 11, 2020
@eaglus eaglus changed the title useWindowScroll may lose window scroll change at first mount useWindowScroll may lose window scroll change at mount Dec 11, 2020
eaglus added a commit to eaglus/react-use that referenced this issue Jan 13, 2021
eaglus added a commit to eaglus/react-use that referenced this issue Mar 11, 2021
…ich#1699

Update tests/useWindowScroll.test.tsx

Co-authored-by: Mathias <mathiassoeholm@gmail.com>

fix for useWindowScroll may lose window scroll change at mount streamich#1699, fixes for review by mathiassoeholm

Update tests/useWindowScroll.test.tsx

Co-authored-by: Mathias <mathiassoeholm@gmail.com>
xobotyi added a commit that referenced this issue Mar 11, 2021
fix for useWindowScroll may lose window scroll change at mount #1699
@xobotyi xobotyi closed this as completed Mar 18, 2021
kamranayub pushed a commit to kamranayub/react-use that referenced this issue May 28, 2021
…ich#1699

Update tests/useWindowScroll.test.tsx

Co-authored-by: Mathias <mathiassoeholm@gmail.com>

fix for useWindowScroll may lose window scroll change at mount streamich#1699, fixes for review by mathiassoeholm

Update tests/useWindowScroll.test.tsx

Co-authored-by: Mathias <mathiassoeholm@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants