prettier-eslint icon indicating copy to clipboard operation
prettier-eslint copied to clipboard

Support TypeScript v4.x

Open opl- opened this issue 5 years ago • 22 comments

Versions:

  • prettier-eslint version: 11.0.0

Problem description:

TypeScript has recently released a new update which bumped the major. Unfortunately, TypeScript doesn't adhere to semver (see microsoft/TypeScript#14116). This unfortunately means that if a user's project depends on TypeScript v4.0, npm/yarn will install a separate TypeScript v3.9 just for this package in order to fulfill the dependency version ranges.

Suggested solution:

Using >=3.9.2 as the dependency version for the typescript package should fix this issue while keeping keeping the option to use older, still compatible TypeScript versions. See https://docs.npmjs.com/files/package.json#dependencies.

Relevant: #391

opl- avatar Sep 13 '20 14:09 opl-

I feel like we could switch this dependency to instead be a peerDependency >=typescript@^3.9.0. What do you think @hamzahamidi ?

The only downside to this would be that, since we're probably not likely to keep track of how each TS version affects this package, we'll eventually have the ceiling reported to us and we'd have to change the range until a fix comes into play. I think this is still better than forcing consumers into multiple versions of TS

kylemh avatar Sep 20 '20 02:09 kylemh

It's a good idea to give flexibility to the user, but I don't know the effect on the package @typescript-eslint/parser

hamzahamidi avatar Sep 20 '20 08:09 hamzahamidi

They follow typescripts release cycle to avoid confusion, so we'd just need to update that dependency as well.

One very interesting question is... where do we stand in regards to SemVer on this?

Should we bump a major if we upgrade typescript and @typescript-eslint/parser 🤔 even weirder, should we be paying attention to every release to follow SemVer?

We could also probably change the way this library works in terms of... maybe we list the key dependencies (eslint, prettier, typescript, @typescript-eslint/parser) as peerDependencies and advertise that we only offer bug support for pairings that our devDependencies are listed as (by the end of Hacktoberfest, that would look like v7, v2, v4, and v4 respectively)?

Could drastically lower the amount of activity in this project's issues/PRs...

kylemh avatar Sep 20 '20 08:09 kylemh

If we make it a peer dependency. What should we use for the tests ? Should we only use the latest version? If however we only upgrade typescript & @typescript-eslint/parser to version 4 we should treat it as major version.

hamzahamidi avatar Sep 20 '20 09:09 hamzahamidi

@hamzahamidi keep in mind neither of those repositories follow SemVer. There aren't breaking changes in v4.

And yeah, I think latest for our own repo is fine.

kylemh avatar Sep 20 '20 16:09 kylemh

Note https://github.com/prettier/prettier-eslint/issues/133 and the decision zimme reached, but I respectfully disagree, since we have 3 years of other issues to point to as evidence.

kylemh avatar Sep 23 '20 02:09 kylemh

If problems are actually solved by moving prettier amd eslint into peer dependencies I don't have a problem with that.

As we do use prettier and eslint in the closest node_modules from the directory where prettier-eslint is run. Maybe it actually makes sense to have the dependencies as peer dependencies to make sure you have a copy of prettier and eslint there. However Im not sure this will actually solve the problems people are facing as if they want to use another version they can just manually install prettier and eslint in their projects and those versions will be used.

Maybe we shouldnt be using our current approch of resolving prettier and eslint.

The only way to be sure of the best way is probably to try in alpha releases

zimme avatar Sep 23 '20 06:09 zimme

My guess would be that a large amount of people that use this dependency aren't aware of the ability to use eslint with the prettier plugin to format their code base. There may be some other use-case that I am not aware of, but for those users - if we instruct them to install this via yarn add -D prettier-eslint eslint prettier - there should be no issue.

The bigger hope from me is that this shrugs off this library's responsibility for ensuring the two pair well. Instead, we're just the system of enforcing: lint first, format second.

We'll release its a breaking alpha.

kylemh avatar Sep 23 '20 06:09 kylemh

The reason I think, could be wrong here, the decision was made to make sure the two play nice is that prettier-eslint isnt just providing format first, auto fix second, but it also provides functionality to infer prettier options from eslint configs.

zimme avatar Sep 23 '20 06:09 zimme

Awesome. I'll write that in the README to explain the differences.

kylemh avatar Sep 23 '20 06:09 kylemh

Here's another issue that peerDeps would solve:

https://stackoverflow.com/questions/55807329/why-eslint-throws-no-unused-vars-for-typescript-interface

There's several subtle errors that can't be fixed until prettier-eslint updates its version of @typescript-eslint/parser. In this case, I'm getting no-unused-vars errors because prettier-eslint currently hard pins to @typescript-eslint/[email protected] and there's no way to tell node to use a different version, even if you install @typescript-eslint/parser to a specific version.

npm ls @typescript-eslint/parser
├─┬ @typescript-eslint/[email protected]        <--- I have 4.15.2 installed
│ └── @typescript-eslint/[email protected] deduped
├── @typescript-eslint/[email protected]
├─┬ [email protected]
│ └─┬ [email protected]
│   └── @typescript-eslint/[email protected] <--- Yet there's a 1.13.0 here...
└─┬ [email protected]
  └── @typescript-eslint/[email protected] <--- AND a 3.10.1 here...  :(

It would be fine if prettier-eslint team was on top of every release, but historically it hasn't been. I'm not trying to throw blame or anything, just making a point that peerDeps would at least allow users to solve incompatibility users by pinning their own version lest be stuck with the version that prettier-eslint decided.

sarendipitee avatar Feb 28 '21 03:02 sarendipitee

Couldn't resolutions solve your problem?

kylemh avatar Feb 28 '21 04:02 kylemh

@kylemh Relying on a yarn only construct isn't exactly ideal 🤔

sarendipitee avatar Feb 28 '21 07:02 sarendipitee

https://github.com/rogeriochaves/npm-force-resolutions just trying to give you a workaround for now.

@zimme I haven't used prettier-eslint for some time, but with the advent of https://github.com/prettier/eslint-config-prettier/pull/175 I'm wondering if the library should even exist anymore.

kylemh avatar Feb 28 '21 07:02 kylemh

I don't really use this library any longer as I don't need my prettier setting being inferred from my eslint settings, which is what this library provides over just using prettier && eslint --fix. So there is still a "need" for this package but the recommended way of using prettier with eslint is to actually use eslint-config-prettier which disables all eslint rules that conflict with prettier.

zimme avatar Feb 28 '21 08:02 zimme

I think it would be a good idea to update the readme with a recommendation to use prettier, eslint and eslint-config-prettier directly unless you want the prettier config to be changed based on your eslint config.

zimme avatar Feb 28 '21 08:02 zimme

I guess there's also a use case for this lib in editors which don't support multiple linter/formatters on one file type

zimme avatar Feb 28 '21 08:02 zimme

As for this specific issue. I do believe we walt to changed all the typescript/vue/other parsers to optional dependencies and check if thery're available and only use them in those cases.

As well as loosening the version ranges on those dependencies.

zimme avatar Feb 28 '21 08:02 zimme

It didn't seem like optionalDependencies was what we wanted.

https://docs.npmjs.com/cli/v7/configuring-npm/package-json#optionaldependencies

Entries in optionalDependencies will override entries of the same name in dependencies, so it's usually best to only put in one place.


Gave it a whirl though: https://github.com/prettier/prettier-eslint/pull/505 👀

kylemh avatar Feb 28 '21 19:02 kylemh

Quick follow up: I switched to yarn and tried resolutions:

[1/4] 🔍  Resolving packages...
warning Resolution field "@typescript-eslint/[email protected]" is incompatible with requested version "@typescript-eslint/parser@^3.0.0"
warning Resolution field "@typescript-eslint/[email protected]" is incompatible with requested version "@typescript-eslint/parser@^1.10.2"

So that's a no go. I think @kylemh's #505 is the way to go moving forward.


Or, is this library just not recommended to be used anymore? Should it be deprecated in favor of an eslint --fix route?

sarendipitee avatar Feb 28 '21 19:02 sarendipitee

@JonDum the warnings are expected, but you should still end up getting the latest version. You can verify by looking at the resolved version in the lockfile.

Also, see my README edits in #505... See if that helps you make a decision re: this or eslint-config-prettier.

kylemh avatar Feb 28 '21 19:02 kylemh

I guess there's also a use case for this lib in editors which don't support multiple linter/formatters on one file type

I use this library for this reason. I have a Visual Studio Code plugin I wrote before the Prettier extension supported ESLint formatting. Plus people have flocked to mine because it's easier to setup.

idahogurl avatar Apr 05 '21 15:04 idahogurl

close in favor of #696

JounQin avatar Aug 15 '22 04:08 JounQin