-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Repo: eslint-plugin should move typescript from devDependencies to dependencies #7411
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
Comments
See https://typescript-eslint.io/users/dependency-versions/#version-warning-logs:
Is there any practical reason you'd want |
I'm working on a build tool that looks at dependencies to see what's needed at run time.
|
Ok, it looks like we need to be smart about also looking at "peerDependencies". However, "typescript" doesn't live there either? and it does not seem to be optional as marked in peerDependenciesMeta |
This sounds like a lot of theoretical "it would be nice to" ideas. If you can demonstrate any concrete issue arising from the way the packages are set up, we'd be happy to discuss fixing. This is why we have so many issue templates. They ask for the info needed to provide enough evidence to make changes. Please use the appropriate issue template next time. 🙂 |
Just want to note that the Typescript compiler API is not stable yet per the Typescript team (https://github.com/microsoft/TypeScript/wiki/Using-the-Compiler-API) so it's not necessarily a the 8000 oretical problem that you're relying on symbols that aren't guaranteed stable across all versions of typescript. |
Some package managerswill install peer dependencies for you if they're missing. In either case both behaviours are not what we want because there are many usecases where framework authors include our plugin somewhat conditionally. For example If we were to use a true peer dependency then users of CRA that don't use TS would get a warning telling them they haven't installed TS. Which is obviously a bad DevX and leads to people complaining! So we don't want to have it as a peer dependency for that reason. However we also need to signal to package managers that we do actually import typescript at runtime - modern package managers may throw a runtime error if you attempt to import a package you don't depend on! This is why we don't list a peer dependency. Ut instead use typescript-eslint/packages/eslint-plugin/package.json Lines 94 to 98 in 35adf7d
This is respected by package managers so they will let us import typescript at runtime. |
We know full well that typescript's API changes from version to version and we are very careful about avoiding using new APIs that aren't available in old versions. We clearly define the allowed versions of TS in our docs and we also have runtime warnings to help inform users if they're on an unsupported version. We don't yet have testing as this is a difficult problem to solve for #1752 |
Suggestion
per https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/package.json#L88, typescript should be in dependencies, not devDependencies because typescript is imported at runtime per https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/util/astUtils.ts#L2
The text was updated successfully, but these errors were encountered: