Repo: eslint-plugin should move typescript from devDependencies to dependencies

[kwasimensah]

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

[JoshuaKGoldberg]

See https://typescript-eslint.io/users/dependency-versions/#version-warning-logs:

Note that our packages have an open peerDependency requirement in order to allow for experimentation on newer/beta versions of TypeScript.

Is there any practical reason you'd want typescript listed in dependencies?

[kwasimensah]

I'm working on a build tool that looks at dependencies to see what's needed at run time.

Looking at https://nodejs.org/es/blog/npm/peer-dependencies, the fact that https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/util/astUtils.ts#L2 imports typescript seems like it needs something stronger than peer dependencies? You also might want to be opinionated about which version of Typescript is used here because the symbols you're relying on may change/disappear between versions.

[kwasimensah]

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

[JoshuaKGoldberg]

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. 🙂

[kwasimensah]

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 theoretical problem that you're relying on symbols that aren't guaranteed stable across all versions of typescript.

[bradzacher]

Some package managerswill install peer dependencies for you if they're missing.
Others will log a warning telling you 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 create-react-app includes us conditionally and then based on the user's setup may or may not install typescript and also include us in the eslint config.

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 peerDependenciesMeta to declare our requirement.

typescript-eslint/packages/eslint-plugin/package.json

Lines 94 to 98 in 35adf7d

	"peerDependenciesMeta": {
	"typescript": {
	"optional": true
	}
	},

This is respected by package managers so they will let us import typescript at runtime.

[bradzacher]

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.
https://typescript-eslint.io/users/dependency-versions#typescript

We don't yet have testing as this is a difficult problem to solve for #1752