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

[no-require-imports] require created by createRequire should be ignored #3713

Closed
3 tasks done
JounQin opened this issue Aug 6, 2021 · 4 comments · Fixed by #3871
Closed
3 tasks done

[no-require-imports] require created by createRequire should be ignored #3713

JounQin opened this issue Aug 6, 2021 · 4 comments · Fixed by #3871
Labels
enhancement New feature or request package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@JounQin
Copy link
Contributor

JounQin commented Aug 6, 2021

  • I have tried restarting my IDE and the issue persists.
  • I have updated to the latest version of the packages.
  • I have read the FAQ and my problem is not listed.

Repro

{
  "rules": {
    "@typescript-eslint/no-require-imports": 2
  }
}
import { createRequire } from 'module'

const require = createRequire()

require('remark-preset-prettier')

Expected Result

No error

Actual Result

error  A `require()` style import is forbidden   @typescript-eslint/no-require-imports

Additional Info

Versions

package version
@typescript-eslint/eslint-plugin 4.29.0
@typescript-eslint/parser 4.29.0
TypeScript 4.3.5
ESLint 7.32.0
node 12.22.1
@JounQin JounQin added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Aug 6, 2021
@JounQin
Copy link
Contributor Author

JounQin commented Aug 6, 2021

Or, that rule should only care about global reference of require.

What means const require = (input: string) => {}; require('') should just be fine?

@bradzacher bradzacher added enhancement New feature or request and removed triage Waiting for maintainers to take a look labels Aug 6, 2021
@rafaelss95
Copy link
Contributor

Since this rule simply checks the AST, which means there's no type information, there is no way it can know if it's a "global require" or not and so in this case, IMHO the best you can do is disable it with a comment.

@bradzacher
Copy link
Member

Through the use of scope analysis you can do accurate detection of cases like this!

If a variable has no declarations, then it's a global variable. So when we inspect a require call, we can just get the scope analysis variable and for the name in the scope and check if has a declaration. If it does, it's a local variable like in OP. If not then it's a global variable.

@rafaelss95
Copy link
Contributor

rafaelss95 commented Sep 12, 2021

Oh yeah. Sorry, I completely forgot about Scope 🤦‍♂️

I've just opened a PR for this (#3871).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants