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

Try to prevent zooming NaN errors #2906

Merged
merged 2 commits into from
Oct 24, 2017
Merged

Conversation

fbarl
Copy link
Contributor

@fbarl fbarl commented Oct 23, 2017

Should fix #2858.

Although I couldn't find the exact cause (the problem is not really reproducible, there seems to be a ~0.5% chance of running into a race condition), I believe that one of the two measures will make it vanish:

Changes
  • Set initial viewport (width, height) to (0, 0) in the global state. That one gets set to proper values once App component mounts, but we want to make sure it's a numerical value (vs. undefined) before in case the zooming selectors which depend on it pick it up before the app is mounted.
  • Make sure NaN values are never cached. That means that even if we hit the same race condition, it should fix itself in the next render loop because the default zoom values will be used again instead of the cached NaN. So if this error does happen again, I expect it would just be thrown once and then the zooming and panning would work normally again.

@fbarl fbarl self-assigned this Oct 23, 2017
@fbarl fbarl requested a review from rndstr October 23, 2017 15:01
Copy link
Contributor

@rndstr rndstr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM! 💯

@fbarl fbarl merged commit 27fea40 into master Oct 24, 2017
@dlespiau dlespiau deleted the 2858-prevent-zooming-nan-errors branch November 2, 2017 12:28
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

Successfully merging this pull request may close these issues.

Zoom broke once on load with translate(NaN, NaN) error
2 participants