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

Adjust no-unsafe rule to handle all unsafe life-cycle methods #2075

Merged
merged 2 commits into from Dec 11, 2018

Conversation

sergei-startsev
Copy link
Contributor

The original PR has been broken down to 2 parts, the changes related to no-unsafe rule have been moved to this PR.

Summary

  • no-unsafe rule has been adjusted to handle all unsafe life-cycle methods including their aliases. Developers who want to avoid using of legacy lifecycle methods can enable it today.
  • Instructions on updating components have been adjusted in no-unsafe rule to be consistent with React runtime warnings.
  • Documentation pages have been updated.
  • Corresponding unit tests have been updated/added.

- Adjust `no-unsafe` rule to handle all unsafe life-cycle methods including their aliases.
- Add instructions on updating components to be consistent with React runtime warnings.
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

It seems like we could avoid a breaking change by adding the non-prefixed methods behind an option (and later, enable that option by default as a breaking change).

lib/rules/no-unsafe.js Outdated Show resolved Hide resolved
Avoid breaking changes by adding the non-prefixed methods behind the option
@sergei-startsev
Copy link
Contributor Author

@ljharb The breaking changes are hidden behind checkAliases option.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Thanks, this looks great!

Copy link
Collaborator

@EvHaus EvHaus left a comment

Choose a reason for hiding this comment

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

LGTM. Really like the new instructions of what method to migrate to. Good touch!

@ljharb ljharb merged commit 4583342 into jsx-eslint:master Dec 11, 2018
@ljharb ljharb added the rule label Dec 11, 2018
This was referenced Jan 4, 2019
@ghost ghost mentioned this pull request Jan 12, 2019
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

3 participants