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

Typescript peer dependency should not be needed #467

Closed
rlindner81 opened this issue Nov 6, 2019 · 15 comments
Closed

Typescript peer dependency should not be needed #467

rlindner81 opened this issue Nov 6, 2019 · 15 comments

Comments

@rlindner81
Copy link

I would like to use this plugin in a project without any typescript, why does it have a peer dependency?
Going from 23.0.1 to 23.0.2 seems to have added this for me.

├─┬ eslint-plugin-jest@23.0.1
│ └─┬ @typescript-eslint/experimental-utils@2.5.0
│   ├── @types/json-schema@7.0.3
│   ├─┬ @typescript-eslint/typescript-estree@2.5.0
│   │ ├─┬ debug@4.1.1
│   │ │ └── ms@2.1.2
│   │ ├── glob@7.1.4 deduped
│   │ ├── is-glob@4.0.1 deduped
│   │ ├── lodash.unescape@4.0.1
│   │ └── semver@6.3.0 deduped
│   └── eslint-scope@5.0.0 deduped
├─┬ eslint-plugin-jest@23.0.2
│ └─┬ @typescript-eslint/experimental-utils@2.6.1
│   ├── @types/json-schema@7.0.3
│   ├─┬ @typescript-eslint/typescript-estree@2.6.1
│   │ ├─┬ debug@4.1.1
│   │ │ └── ms@2.1.2
│   │ ├── glob@7.1.5 deduped
│   │ ├── is-glob@4.0.1 deduped
│   │ ├── lodash.unescape@4.0.1
│   │ ├── semver@6.3.0
│   │ ├─┬ tsutils@3.17.1
│   │ │ └── tslib@1.10.0 deduped
│   │ └── UNMET PEER DEPENDENCY typescript@>=2.8.0 || >= 3.2.0-dev || >= 3.3.0-dev || >= 3.4.0-dev || >= 3.5.0-dev || >= 3.6.0-dev || >= 
@SimenB
Copy link
Member

SimenB commented Nov 7, 2019

It's marked as optional - have you updated your version of npm?

https://github.com/typescript-eslint/typescript-eslint/blob/c8fe51575d743ad317a09c18658c79d45059412b/packages/typescript-estree/package.json#L68-L72

See typescript-eslint/typescript-eslint#990


Closing this as I don't think it's an issue. Happy to reopen if it is, though

@SimenB SimenB closed this as completed Nov 7, 2019
@rlindner81
Copy link
Author

No, these trees are taken from the same npm version 6.9.0...
This is an issue for us unfortunately. Despite the optional marking npm ls will return a non-zero errorcode for the "missing" peer dependency which is interpreted as having an incomplete dependency hierarchy and stops a pipeline I would like to use.

% npm -v
6.9.0
% npm ls >/dev/null
npm ERR! peer dep missing: typescript@>=2.8.0 || >= 3.2.0-dev || >= 3.3.0-dev || >= 3.4.0-dev || >= 3.5.0-dev || >= 3.6.0-dev || >= 3.6.0-beta || >= 3.7.0-dev || >= 3.7.0-beta, required by tsutils@3.17.1
% echo $?          
1

The change seems to have happened in typescript-eslint/typescript-estree of course, but for them I feel that a typescript peer dependency is more reasonable. Whereas jest for our project is pretty much unrelated to typescript.

@SimenB
Copy link
Member

SimenB commented Nov 8, 2019

npm 6.9 is pretty old, you need at least 6.11: https://github.com/npm/cli/releases/tag/v6.11.0

@rlindner81
Copy link
Author

Ok, thanks for the info

@SimenB
Copy link
Member

SimenB commented Nov 8, 2019

That worked? If not, should probably raise an issue with typescript-eslint as they explicitly support using their utils without requiring TS by consumers

@rlindner81
Copy link
Author

Well in our exact setup it does not help, but that's fair enough. I will try to move our ops team to update the pipeline's npm version, which will happen eventually. Thanks @SimenB

@vieira
Copy link

vieira commented Nov 20, 2019

Hello,

This also produces a warning with yarn (1.19.1) which I would like to avoid without adding a dependency to typescript.

warning "eslint-plugin-jest > @typescript-eslint/experimental-utils > @typescript-eslint/typescript-estree > tsutils@3.17.1" has unmet peer dependency "typescript@>=2.8.0 || >= 3.2.0-dev || >= 3.3.0-dev || >= 3.4.0-dev || >= 3.5.0-dev || >= 3.6.0-dev || >= 3.6.0-beta || >= 3.7.0-dev || >= 3.7.0-beta".

@SimenB
Copy link
Member

SimenB commented Nov 20, 2019

Please open up an issue with typescript-eslint - it was added in typescript-eslint/typescript-eslint#1120

lasseborly added a commit to danskernesdigitalebibliotek/ddb-react that referenced this issue Dec 11, 2019
The specific TS dev dep is required to have the updated eslint dep not throw any warnings.

jest-community/eslint-plugin-jest#467
kasperg pushed a commit to danskernesdigitalebibliotek/ddb-react that referenced this issue Jan 7, 2020
The specific TS dev dep is required to have the updated eslint dep not throw any warnings.

jest-community/eslint-plugin-jest#467
@MatteoGabriele
Copy link

Hi there!
I've seen lots of issues about this and mostly closed issues and yet the warning is still there: why this warning about typescript? why @typescript-eslint/experimental-utils is a dependency and not a devDependency?

I'm using yarn 1.21.1 and eslint-plugin-jest ^23.0.4 and still see this all the time I install.
is there a fix for this?

thanks a lot for your time

@SimenB
Copy link
Member

SimenB commented Jan 9, 2020

This is the current state: typescript-eslint/typescript-eslint#1287 (comment)

Might be worth opening up an issue with tsutils and asking if they'd consider making the peer optional?

@MatteoGabriele
Copy link

Yeah i read that too, bit shouldn't this package have it has a devDependency? Wouldn't that solve it?

@SimenB
Copy link
Member

SimenB commented Jan 9, 2020

We already have it as a dev dep, but that's not included when you install this module.

typescript is never required at runtime(*), so it's completely safe to ignore the warning. I understand it's annoying though, and the solution is for modules who have it as peer to mark it as optional if they do not need it at runtime. tsutils might need to refactor quite a bit in order to make that true in all cases though, so they might not be open to the change. Won't know without asking them 🙂

  • as of now - this might change, in which case we need to figure something out.

@MatteoGabriele
Copy link

I do see it in the package.json as a dependency: https://github.com/jest-community/eslint-plugin-jest/blob/master/package.json#L39

thanks for your quick feedback btw!

@vieira
Copy link

vieira commented Jan 22, 2020

@SimenB Would moving @typescript-eslint/experimental-utils from dependencies to devDependencies as @MatteoGabriele suggested solve it?

I understand that it is safe to ignore this particular warning, however I am afraid that if we start having this kind of "safe" warnings people will simply start to ignore all warnings, so I would like to avoid them if possible.

@G-Rath
Copy link
Collaborator

G-Rath commented Jan 23, 2020

@vieira

@SimenB Would moving @typescript-eslint/experimental-utils from dependencies to devDependencies as @MatteoGabriele suggested solve it?

It would solve the warning occuring, but create the new problem of breaking the entire package, as you'd have removed a critical dependency :)

andreineculau added a commit to ysoftwareab/eslint-plugin-y that referenced this issue Jan 27, 2021
…mmit description for WTF?!

See
* typescript-eslint/typescript-eslint#1120
* jest-community/eslint-plugin-jest#467
* typescript-eslint/typescript-eslint#1287

eslint-plugin-jest
depends on typescript-eslint/experimental-utils, which
depends on typescript-eslint/typescript-estree, which
depends on tsutils, which
depends on typescript as a peer dependency

tsutils has been added as a dependency to
typescript-eslint/typescript-estree from 2.6.0 to 2.6.1 (yes, a patch bump!)
in typescript-eslint/typescript-eslint#1120

I'm guessing the peer dependency requirement went unchecked because
typescript was already in their devDependencies for obvious reaons,
but also tsutils was used in other packages in the monorepo,
making it a trivial change.

The reactions to the change were

1. "ignore the warning" typescript-eslint/typescript-eslint#1287 (comment)
So tens, hundreds, thousands, more people should not pay attention to a warning.
In my case, we actually fail the build process if there are unmet peer dependencies.
Since many repos depend on eslint, this means that all those repos will start failing. So ignore I can't.

2. use yarn or npm 6.11+ which introduced peerDependenciesMeta
which marks tsutils' peerDependency on typescript as optional
jest-community/eslint-plugin-jest#467 (comment)
typescript-eslint/typescript-eslint#990
Except when running npm 6.12.1 and 6.14.11, I still get that error...

So that's why.
andreineculau added a commit to ysoftwareab/eslint-plugin-y that referenced this issue Jan 28, 2021
…mmit description for WTF?!

See
* typescript-eslint/typescript-eslint#1120
* jest-community/eslint-plugin-jest#467
* typescript-eslint/typescript-eslint#1287

eslint-plugin-jest
depends on typescript-eslint/experimental-utils, which
depends on typescript-eslint/typescript-estree, which
depends on tsutils, which
depends on typescript as a peer dependency

tsutils has been added as a dependency to
typescript-eslint/typescript-estree from 2.6.0 to 2.6.1 (yes, a patch bump!)
in typescript-eslint/typescript-eslint#1120

I'm guessing the peer dependency requirement went unchecked because
typescript was already in their devDependencies for obvious reaons,
but also tsutils was used in other packages in the monorepo,
making it a trivial change.

The reactions to the change were

1. "ignore the warning" typescript-eslint/typescript-eslint#1287 (comment)
So tens, hundreds, thousands, more people should not pay attention to a warning.
In my case, we actually fail the build process if there are unmet peer dependencies.
Since many repos depend on eslint, this means that all those repos will start failing. So ignore I can't.

2. use yarn or npm 6.11+ which introduced peerDependenciesMeta
which marks tsutils' peerDependency on typescript as optional
jest-community/eslint-plugin-jest#467 (comment)
typescript-eslint/typescript-eslint#990
Except when running npm 6.12.1 and 6.14.11, I still get that error...

So that's why.
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

No branches or pull requests

5 participants