Skip to content

eslint-plugin should not import from typescript-estree #705

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.

Sign up for GitHub

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

Jump to bottom
Closed
mikeharder opened this issue Jul 15, 2019 · 0 comments · Fixed by #706
Closed

eslint-plugin should not import from typescript-estree #705

mikeharder opened this issue Jul 15, 2019 · 0 comments · Fixed by #706
Labels
bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@mikeharder
Copy link
Contributor

mikeharder commented Jul 15, 2019 •
edited
Loading

Repro
@typescript-eslint/eslint-plugin should not import from @typescript-eslint/typescript-estree, since it does not have a direct dependency on this package. Instead, it should only import from @typescript-eslint/experimental-utils.

This issue is hidden when using a package manager which flattens dependencies like npm, since @typescript-eslint/parser does have a dependency on @typescript-eslint/typescript-estree, and @typescript-eslint/eslint-plugin and @typescript-eslint/parser are typically used together.

However, when using a package manager like pnpm which does not flatten dependencies, @typescript-eslint/eslint-plugin fails with the following error:

Error: Cannot find module '@typescript-eslint/typescript-estree'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
    at Function.Module._load (internal/modules/cjs/loader.js:562:25)
    at Module.require (internal/modules/cjs/loader.js:690:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (D:\Git\typescript-eslint-test\node_modules\.registry.npmjs.org\@typescript-eslint\eslint-plugin\1.12.0_db854cf46887ef4aa7b9323cccc417a5\node_modules\@typescript-eslint\eslint-plugin\dist\rules\prefer-readonly.js:17:29)
    at Module._compile (internal/modules/cjs/loader.js:776:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:787:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)

Repro steps:

git clone https://github.com/mikeharder/typescript-eslint-test

npm install
npm run lint (works fine)

git clean -xdf
npm install -g pnpm
pnpm install
npm run lint (fails)

Versions

package version
@typescript-eslint/eslint-plugin 1.12.0
@typescript-eslint/parser 1.12.0
TypeScript 3.5.3
ESLint 5.16.0
node 10.16.0
npm 6.9.0
pnpm 3.5.7
@mikeharder mikeharder added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels Jul 15, 2019
mikeharder added a commit to mikeharder/typescript-eslint that referenced this issue Jul 15, 2019
@mikeharder
fix(eslint-plugin): add dependency typescript-estree
78b9d7f 
- Fixes typescript-eslint#705
@mikeharder mikeharder mentioned this issue Jul 15, 2019
Merged
@bradzacher bradzacher added bug Something isn't working and removed triage Waiting for team members to take a look labels Jul 15, 2019
@mikeharder mikeharder changed the title eslint-plugin should have a dependency on typescript-estree eslint-plugin should not import from typescript-estree Jul 16, 2019
@mikeharder mikeharder mentioned this issue Jul 16, 2019
Merged
mikeharder added a commit to Azure/azure-sdk-for-js that referenced this issue Jul 16, 2019
@mikeharder
Fix dependencies for @typescript-eslint/eslint-plugin (#4327)
7c7e810 
- @typescript-eslint/eslint-plugin@1.12.0 introduced an implicit dependency on @typescript-eslint/typescript-estree
  - This should be fixed in a future release of @typescript-eslint/eslint-plugin
  - typescript-eslint/typescript-eslint#705
bradzacher pushed a commit that referenced this issue Jul 16, 2019
@mikeharder @bradzacher
fix(eslint-plugin): remove imports from typescript-estree (#706)
ceb2d32 
Fixes #705
@mikeharder mikeharder mentioned this issue Jul 23, 2019
Merged
mikeharder added a commit to Azure/azure-sdk-for-js that referenced this issue Jul 23, 2019
@mikeharder
Revert "Fix dependencies for @typescript-eslint/eslint-plugin (#4327)" (
e80c96e 
#4390)

- @typescript-eslint/eslint-plugin@1.12.0 introduced an implicit dependency on @typescript-eslint/typescript-estree
- Fixed in @typescript-eslint/eslint-plugin@1.13.0
- typescript-eslint/typescript-eslint#705
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug Something isn't working package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(eslint-plugin): remove imports from typescript-estree mikeharder/typescript-eslint
2 participants
@bradzacher @mikeharder