Skip to content

Rule Blocklist update #251

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

Merged
merged 1 commit into from
Apr 11, 2017
Merged

Rule Blocklist update #251

merged 1 commit into from
Apr 11, 2017

Conversation

pointlessone
Copy link
Contributor

As per comment in #231, here's update to rule blocklist.

Remove:

  • import/extensions — it does not fail when can not resolve module, uses literal value instead
  • import/no-absolute-path — does not use module resolution, checks whether literal value starts with "/". The mentioned babel-module issue does not appear to be related to this specific rule but a misconfiguration of ESLint.

Add:

  • import/no-restricted-paths — relies on absolutely resolved module paths
  • node/no-hide-code-modules — relies on module resolution, also deprecated

@pointlessone
Copy link
Contributor Author

Here's full list of resolve mentions in the rules source.

  • eslint-config-airbnb-base/rules/imports.js — aggregate
  • eslint-config-airbnb-base/rules/style.js — comment
  • eslint-config-airbnb/rules/react.js — aggregate
  • eslint-config-simplifield/node_modules/eslint-plugin-angular/rules/deferred.js — var
  • eslint/lib/rules/global-require.js — doesn’t do module resolution
  • eslint/lib/rules/no-alert.js — doesn’t do module resolution
  • eslint/lib/rules/no-loop-func.js — doesn’t do module resolution
  • eslint/lib/rules/no-path-concat.js — string literal
  • eslint/lib/rules/no-unmodified-loop-condition.js — doesn’t do module resolution
  • eslint/lib/rules/no-unused-vars.js — doesn’t do module resolution
  • eslint/lib/rules/no-use-before-define.js — doesn’t do module resolution
  • eslint/lib/rules/prefer-const.js — comment
  • eslint/lib/rules/prefer-promise-reject-errors.js — string literal
  • eslint-plugin-angular/rules/deferred.js — comments, string literal
  • eslint-plugin-filenames/lib/rules/match-exported.js — path, not module
  • eslint-plugin-filenames/lib/rules/match-regex.js — path, not module
  • eslint-plugin-filenames/lib/rules/no-index.js — path, not module
  • eslint-plugin-flowtype/dist/rules/defineFlowType.js — object field
  • eslint-plugin-import/lib/rules/extensions.js — doesn’t fail when can’t resolve, uses specified string to find extension
  • eslint-plugin-import/lib/rules/no-duplicates.js — doesn’t fail when can’t resolve, uses specified string to identify modules
  • eslint-plugin-import/lib/rules/no-internal-modules.js — doesn’t fail when can’t resolve, tries other methods first
  • eslint-plugin-import/lib/rules/no-restricted-paths.js
  • eslint-plugin-import/lib/rules/no-unresolved.js
  • eslint-plugin-inferno/lib/rules/no-unused-prop-types.js — comments, local function name
  • eslint-plugin-inferno/lib/rules/prop-types.js — comments, local function name
  • eslint-plugin-jsx-a11y/lib/rules/aria-proptypes.js — comment
  • eslint-plugin-jsx-a11y/src/rules/aria-proptypes.js — comment
  • eslint-plugin-node/lib/rules/no-hide-core-modules.js
  • eslint-plugin-node/lib/rules/no-unpublished-bin.js — path resolution
  • eslint-plugin-node/lib/rules/shebang.js — path resolution
  • eslint-plugin-promise/rules/lib/is-promise.js — unrelated string literal
  • eslint-plugin-promise/rules/no-return-wrap.js — comments, var names, and string literals
  • eslint-plugin-promise/rules/param-names.js — string literals
  • eslint-plugin-react/lib/rules/no-unused-prop-types.js — comments, local function name
  • eslint-plugin-react/lib/rules/prop-types.js — comments, local function name
  • eslint-plugin-react/lib/rules/require-default-props.js — comments, local function names, and string literals

Remove:

* import/extensions - it does not fail when can not resolve module, uses
  literal value instead
* import/no-absolute-path - does not use module resolution, checks
  whether literal value starts with "/"

Add:

* import/no-restricted-paths - relies on absolutely resolved module
  paths
* node/no-hide-code-modules - relies on module resolution, also
  deprecated
@pointlessone pointlessone force-pushed the rule-blocklist-update branch from ed04cbc to ea23162 Compare April 10, 2017 16:02
Copy link
Contributor

@dblandin dblandin left a comment

Choose a reason for hiding this comment

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

LGTM!

@pointlessone
Copy link
Contributor Author

@efueger I would like to have your feedback regarding the import/no-absolute-path rule. I don't want to break stuff.

@efueger
Copy link
Member

efueger commented Apr 10, 2017

@pointlessone - I think @dblandin would be a better judge of whether that should be included or not.

  • My suggestion was based on "hey, it sounds similar and customers bump into it, so maybe it should be included" vs really understanding what it's checking.

@dblandin
Copy link
Contributor

@pointlessone's explanation of that rule makes sense to me. Sounds like it doesn't rely on module resolution but instead checks the string for absolute paths.

@dblandin dblandin merged commit af1fe5e into master Apr 11, 2017
@dblandin dblandin deleted the rule-blocklist-update branch April 11, 2017 14:55
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.

4 participants