-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Fix @typescript-eslint typescript peer dependency warning #6859
Conversation
Doh. Not quite this simple. Still a couple warnings left it looks like when checking out the CI logs:
|
packages/react-scripts/package.json
Outdated
@@ -30,8 +30,8 @@ | |||
"dependencies": { | |||
"@babel/core": "7.4.3", | |||
"@svgr/webpack": "4.1.0", | |||
"@typescript-eslint/eslint-plugin": "1.6.0", | |||
"@typescript-eslint/parser": "1.6.0", | |||
"@typescript-eslint/eslint-plugin": "1.7.1-alpha.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want to ship an alpha version just to remove some warnings. Can we wait until this is stable and upgrade it after the 3.0 release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can yes. This is a commit after their 1.7.0
release as they release automatically on commit to master.
I've published |
Alright looks like this might be the last one:
|
@ianschmitz can this be moved ahead now that 1.8.0 (and indeed, up through 1.13.0) have been released? Just doing an audit of my project's install warning and came across this. |
736a336
to
2424854
Compare
Still getting warnings :(
|
It's probably worth merging this for 3.1 anyway, what do you think @ianschmitz @iansu? |
Fwiw I don't think using an unsafe pattern known to break under some circumstances (omitting peer dependencies from the manifest) just to remove warnings is a good tradeoff, especially when those warning can already disappear for half the users by using the right feature (npm is also going to implement optional peer dependencies, btw) ... I should probably bring this to the typescript-eslint project, but since I think they made the change based on CRA I wanted to raise it here as well 🙁 |
@arcanis thanks for the additional info. I think we should hold off on this since it doesn't actually solve the problem. If this is something that's going to be addressed directly by package managers then maybe we should just wait for that to happen. |
@ianschmitz this is outdated so can probably be closed |
today is 2020.10.01 and I still got below from a fresh create-react-app project:
|
Agreed. Webpack 5 also includes native pnp support AFAIK, so that will change things. |
Fixes #6834.
Bump
@typescript-eslint
packages to a newer version that have removed thetypescript
peer dependency.