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

[lint] Add a lint rule to disallow Haste imports #25058

Closed
wants to merge 2 commits into from

Conversation

ide
Copy link
Contributor

@ide ide commented May 27, 2019

Summary

This is an ESLint plugin that infers whether an import looks like a Haste module name. To keep the linter fast and simple, it does not look in the Haste map. Instead, it looks for uppercase characters in single-name import paths, since npm has disallowed uppercase letters in package names for a long time. There are some false negatives (e.g. "merge" is a Haste module and this linter rule would not pick it up) but those are about 1.1% of the module names in the RN repo, and unit tests and integration tests will fail anyway once Haste is turned off.

You can disable the lint rule on varying granular levels with ESLint's normal disabling/enabling mechanisms.

Also rewrote more Haste imports so that the linter passes (i.e. fixed lint errors as part of this PR).

Changelog

[General] [Changed] - Add a lint rule to disallow Haste imports

Test Plan

Run yarn lint, yarn flow, and yarn test as a sanity check. Modify an import statement to use a Haste module name and verify the linter reports an error. Do the same with a require call.

@ide ide requested a review from cpojer May 27, 2019 20:54
@ide ide requested a review from hramos as a code owner May 27, 2019 20:54
@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: Expo Partner: Expo Partner labels May 27, 2019
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.

Perfect!

This is an ESLint plugin that infers whether an import looks like a Haste module name. To keep the linter fast and simple, it does not look in the Haste map. Instead, it looks for uppercase characters in single-name import paths, since npm has disallowed uppercase letters in package names for a long time. There are some false negatives (e.g. "merge" is a Haste module and this linter rule would not pick it up) but those are about 1.1% of the module names in the RN repo, and unit tests and integration tests will fail anyway once Haste is turned off.

Test Plan: Run `yarn lint` as a sanity check. Modify an import statement to use a Haste module name and verify the linter reports an error. Do the same with a require call.
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.

@pull-bot
Copy link

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

Generated by 🚫 dangerJS against 0f6f8a6

@cpojer
Copy link
Contributor

cpojer commented May 28, 2019

Doing some internal fiddling right now to make this land. It's gonna look a little bit different from the version of the PR here in terms of how it's integrated into the repo.

@ide
Copy link
Contributor Author

ide commented May 28, 2019

It probably makes sense to publish @react-native-community/eslint-plugin to npm as well. I wasn't able to do that because the permission system seems broken but you might have better luck.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @ide in 33ee6f8.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label May 30, 2019
vovkasm pushed a commit to vovkasm/react-native that referenced this pull request Aug 7, 2019
Summary:
This is an ESLint plugin that infers whether an import looks like a Haste module name. To keep the linter fast and simple, it does not look in the Haste map. Instead, it looks for uppercase characters in single-name import paths, since npm has disallowed uppercase letters in package names for a long time. There are some false negatives (e.g. "merge" is a Haste module and this linter rule would not pick it up) but those are about 1.1% of the module names in the RN repo, and unit tests and integration tests will fail anyway once Haste is turned off.

You can disable the lint rule on varying granular levels with ESLint's normal disabling/enabling mechanisms.

Also rewrote more Haste imports so that the linter passes (i.e. fixed lint errors as part of this PR).

[General] [Changed] - Add a lint rule to disallow Haste imports
Pull Request resolved: facebook#25058

Differential Revision: D15515826

Pulled By: cpojer

fbshipit-source-id: d58a3c30dfe0887f8a530e3393af4af5a1ec1cac
(cherry picked from commit 33ee6f8)
M-i-k-e-l pushed a commit to M-i-k-e-l/react-native that referenced this pull request Mar 10, 2020
Summary:
This is an ESLint plugin that infers whether an import looks like a Haste module name. To keep the linter fast and simple, it does not look in the Haste map. Instead, it looks for uppercase characters in single-name import paths, since npm has disallowed uppercase letters in package names for a long time. There are some false negatives (e.g. "merge" is a Haste module and this linter rule would not pick it up) but those are about 1.1% of the module names in the RN repo, and unit tests and integration tests will fail anyway once Haste is turned off.

You can disable the lint rule on varying granular levels with ESLint's normal disabling/enabling mechanisms.

Also rewrote more Haste imports so that the linter passes (i.e. fixed lint errors as part of this PR).

## Changelog

[General] [Changed] - Add a lint rule to disallow Haste imports
Pull Request resolved: facebook#25058

Differential Revision: D15515826

Pulled By: cpojer

fbshipit-source-id: d58a3c30dfe0887f8a530e3393af4af5a1ec1cac
@ide ide deleted the haste-lint branch April 25, 2020 08:29
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: Expo Partner: Expo Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants