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

Eliminate eslint npm version mismatch warnings and bump some to latest #23969

Closed

Conversation

matthargett
Copy link
Contributor

Summary

I was annoyed by warnings from yarn/npm about eslint peer dependencies not being met, so I dived in to try and get rid of some of them. Sometimes it meant bumping a plugin, but then that plugin needed a newer babel-eslint, so it was a dance.

Some we can't easily update to latest (eslint-plugin-prettier) because the rule format has changed a bit. Happy to do that in this PR if folks think its a good idea. eslint-config-fbjs itself needs to be updated and republished to eliminate the last few warnings.

There are a few new warnings (the repo wasn't linting cleanly for me from the start). I can fix those in this PR, or a separate one, based on people's preferences.

Changelog

[internal] [chore] - Eliminate some peer dependency warnings and bump some eslint packages to latest.

Test Plan

  1. Ran eslint ., verified no major new warnings detected and that seemingly none of the new warnings were false positives.

…st for some of these eslint packages. Some we can't easily update to latest (eslint-plugin-prettier) because the rule format has changed a bit. Happy to do that in this PR if folks agree. eslint-config-fbjs itself needs to be updated and republished to eliminate the last few warnings.
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Sony Partner: Sony Partner labels Mar 15, 2019
@pull-bot
Copy link

pull-bot commented Mar 15, 2019

Warnings
⚠️ 🔒 package.json - Changes were made to package.json. This will require a manual import by a Facebook employee.
Messages
📖

📋 Changelog Format - Did you include a Changelog? A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

CATEGORY may be:
  • General
  • iOS
  • Android

TYPE may be:

  • Added, for new features.
  • Changed, for changes in existing functionality.
  • Deprecated, for soon-to-be removed features.
  • Removed, for now removed features.
  • Fixed, for any bug fixes.
  • Security, in case of vulnerabilities.

MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

Generated by 🚫 dangerJS against d9cf2ef

…pecified some of the package bumps I did in the base directory.
"eslint-plugin-eslint-comments": "^3.1.1",
"eslint-plugin-flowtype": "2.50.3",
"eslint-plugin-jest": "22.4.1",
"eslint-plugin-jsx-a11y": "6.2.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this plugin is targeting web: https://github.com/evcohen/eslint-plugin-jsx-a11y#supported-rules
It's also not enabled in the config's plugin list. I think that can be removed.

For RN there's https://github.com/FormidableLabs/eslint-plugin-react-native-a11y but it stopped receiving support shortly after release and iirc it still lacks some accessibility features introduced in 0.57

Copy link
Contributor

Choose a reason for hiding this comment

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

Whoops, scratch my last comment. I thought it's about the community plugin. eslint-config-fbjs requires the eslint-plugin-jsx-a11y

@cpojer
Copy link
Contributor

cpojer commented Mar 18, 2019

May take a little while to get this landed as the eslint version is tied to the internal version at fb.

…orkaround a random eslintrc in there that was getting picked up.
@matthargett
Copy link
Contributor Author

May take a little while to get this landed as the eslint version is tied to the internal version at fb.

Took it back down to eslint ^5.1.0, since that's what metro repo is at. I submitted another PR to fbjs to align these versions and eliminate more of the warnings as well.

Copy link
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Mar 19, 2019
@facebook-github-bot
Copy link
Contributor

@cpojer merged this pull request in e67aa42.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Sony Partner: Sony Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants