gitKrystan / prettier-plugin-ember-template-tag

A prettier plugin for formatting Ember template tags
MIT License
22 stars 12 forks source link

chore: support Prettier v3 #63

Closed Techn1x closed 1 year ago

Techn1x commented 1 year ago

Aims to resolve https://github.com/gitKrystan/prettier-plugin-ember-template-tag/issues/61

Should be considered a breaking change / major. Will drop support for prettier v2

blocked on https://github.com/gitKrystan/prettier-plugin-ember-template-tag/pull/63/files#r1269162266

evoactivity commented 1 year ago

First TS error, try changing the import to

import { parsers as babelParsers } from 'prettier/plugins/babel';
evoactivity commented 1 year ago

Second TS error is because Options is defined in options.ts instead of using Options from prettier. Prettier exports Options here. You could try importing that Options instead.

Techn1x commented 1 year ago

The import change fixed the first error, thanks! I'll remove that error out of the description.

Second TS error... unfortunately using the prettier options import doesn't quite work. It does satisfy the embed hook type signature, but the code inside the hook expects something more like ParserOptions (you can see functions like printRawText() use options.originalText and options.__inputWasPreprocessed which aren't available on the prettier options export)

Techn1x commented 1 year ago

hey @gitKrystan @ef4 - this seems pretty close to being ready for review / mergable. Anyone able to help out with the Options type issue? I'm a little stumped

ef4 commented 1 year ago

I took a quick look, and it seems like either prettier has a bug in their types or we have a bug in our implementation.

If you look at this:

https://github.com/prettier/prettier/blob/24dfe5afe768b627f09484426b26fca764251225/src/index.d.ts#L466-L487

it seems fishy that print takes options: ParserOptions<T> but embed takes options: Options. Our code was assuming that they both get what prettier 3 calls ParserOptions<T>. It looks like Prettier's types are maintained manually, so it's very possible that they don't match their real implementation.

ef4 commented 1 year ago

There's also a bunch of types in this repo that were trying to model prettier, and they don't match up with the new implementation. For example, that's why eslint is complaining about await estreePlugin.printers.estree. It thinks that's not a promise, but we know that now it is.

Techn1x commented 1 year ago

Thanks! That gives me something to go on...

it seems fishy that print takes options: ParserOptions but embed takes options: Options

I agree. Especially since it wasn't noted in release notes / upgrade docs

Here's where the embed signature was changed https://github.com/prettier/prettier/issues/14553 https://github.com/prettier/prettier/pull/14598

Looking through the prettier code, I can see that their own usages of the embed hook expect options arg to be the more complete ParserOptions, eg https://github.com/fisker/prettier/blob/79d8a3983842d1d8a077f177ed8b983c1c596088/src/language-yaml/embed.js#L3-L13

I think what I'll do is update this PR to expect ParserOptions for the embed arg, ignore the TS error, and I'll open an issue in the prettier repo to confirm.

Techn1x commented 1 year ago

Added issue to prettier here https://github.com/prettier/prettier/issues/15132

Techn1x commented 1 year ago

superseded by https://github.com/gitKrystan/prettier-plugin-ember-template-tag/pull/78/