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

Don't lint against Hooks after conditional throw #14040

Merged
merged 1 commit into from
Oct 31, 2018

Conversation

sophiebits
Copy link
Collaborator

@sophiebits sophiebits commented Oct 30, 2018

Seems like this should be OK. Fixes #14038.

Now when tracking paths, we completely ignore segments that end in a throw.

Copy link
Collaborator

@acdlite acdlite left a comment

Choose a reason for hiding this comment

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

Seems fine, with the caveat that I don't know much about ESLint plugins

@sophiebits
Copy link
Collaborator Author

It's possible that thrownSegments can help me? I don't know.

@sizebot
Copy link

sizebot commented Oct 30, 2018

Details of bundled changes.

Comparing: 169f935...fcd4703

scheduler

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
scheduler.development.js n/a n/a 0 B 19.17 KB 0 B 5.74 KB UMD_DEV
scheduler.production.min.js n/a n/a 0 B 3.16 KB 0 B 1.53 KB UMD_PROD

Generated by 🚫 dangerJS

Seems like this should be OK. Fixes facebook#14038.

Now when tracking paths, we completely ignore segments that end in a throw. In https://eslint.org/docs/developer-guide/code-path-analysis I don't see a way to detect throws other than manually tracking them, so that's what I've done.
@sophiebits sophiebits merged commit 3db8b80 into facebook:master Oct 31, 2018
jetoneza pushed a commit to jetoneza/react that referenced this pull request Jan 23, 2019
Seems like this should be OK. Fixes facebook#14038.

Now when tracking paths, we completely ignore segments that end in a throw.
This was referenced Sep 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants