sheerun / prettier-standard

Formats with Prettier and lints with ESLint+Standard! (✿◠‿◠)
MIT License
868 stars 44 forks source link

Upgrade prettierx #90

Closed havenchyk closed 4 years ago

havenchyk commented 4 years ago

Hey @sheerun

I see there is a breaking change in this update because prettierx dropped typescript from the bundle and expects it as a peerDependency as far as I understand.

Also my goal is to upgrade prettierx to the next version that will be published (after 0.10.0 - current latest version).

This PR is mostly a question, what's the best way to deal with typescript, I don't think that having it as a dependency of prettier-standard makes much sense, so maybe it's better to have it as a peerDependency instead?

Relates to #89

sheerun commented 4 years ago

I actually inline eslint plugins because of peerDependency issues (https://github.com/sheerun/prettier-standard/tree/master/src/vendor) so for sure I don't want to make prettierx peerDependency. I can inline it instead, as well as typescript.

sheerun commented 4 years ago

By inline I mean vendor

havenchyk commented 4 years ago

@sheerun tbh, I didn't get how that "vendors" folder works, iirc I had to install all those packages manually, so it worked like peerDependencies

sheerun commented 4 years ago

It's not something standard. Before I publish this package I go do vendor folder to install packages to vendor/node_modules. This way there are packages within prettier-standard itself and don't need to be installed separately as peerDependencies

sheerun commented 4 years ago

My advise for @brodybits would be to remove typescript as peerDependency, because it gives annoying warning when someone does not need it. Instead you can use something like require.resolve to check if typescript is available and require it conditionally only then.

brodycj commented 4 years ago

use something like require.resolve to check if typescript is available and require it conditionally only then

brodybits/prettierx#63

No promises but I will take a look at this idea.

I think the ultimate solution would be to use TypeScript from an external plugin, see https://github.com/brodybits/prettier-plugin-prettierx-typescript for an example.

brodycj commented 4 years ago

I just fixed prettierx to use peerDependenciesMeta for typescript package, like they did in typescript-estree (see https://github.com/brodybits/prettierx/issues/63#issuecomment-558351490) and just published new release 0.11.0. There are still some warning messages due to other packages, now tracked in brodybits/prettierx#65.

havenchyk commented 4 years ago

@sheerun upgraded prettierx to 0.11 with bugfix I want to include. what do you think about peerDependenciesMeta?

sheerun commented 4 years ago

now it's fine, I've updated it already on master branch, but I'm unable to publish.. something is wrong with npm it seems

sheerun commented 4 years ago

I've contacted npm support, let's wait for them to respond

havenchyk commented 4 years ago

@sheerun seems you already did all the necessary changes, so I'm closing this PR and waiting for new version on npm. Thanks!

sheerun commented 4 years ago

@havenchyk I've just published 16.0.0, can you check if it works for you?

havenchyk commented 4 years ago

@sheerun checked, works like a charm. Thank you! (but I use TypeScript and have typescript package as a dependency) 😄