-
-
Notifications
You must be signed in to change notification settings - Fork 173
Fix plugin compatibility with Prettier v3.7+ #418
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
Conversation
Some older/outdated plugins don’t work with Prettier v3.7
|
Nice. Current solution until this gets released: "prettier-plugin-tailwindcss": "git://github.com:tailwindlabs/prettier-plugin-tailwindcss#ab900fa3ecdca78a2e19647336c503f9bf45e741",
|
|
I spend sometime to debug, seem the root cause is here prettier-plugin-tailwindcss/src/index.ts Line 53 in dd02e91
|
It’s unncessary and causes an incompatibility with Prettier v3.7+ Most likely a left over from when we *also* supported Prettier v2.
That plugin is currently broken when used with Prettier v3.7
0646490 to
b5b2531
Compare
|
|
||
| // @ts-ignore | ||
| return base.printers['svelte-ast'].embed(path, options) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we consider use a proxy here?
const embed = new Proxy(base.printers['svelte-ast'].embed, {
apply(target, thisArgument, argumentsList) {
const [path, options] = argumentsList;
mutateOriginalText(path, options)
return Reflect.apply(target, thisArgument, argumentsList)
}
})just incase svelte add visitorKeys for embed in future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oooh yeah, that's a good idea. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it's worth doing the same for print as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but currently Prettier doesn't support additional keys on it. So up to you.
It’s not strictly necessary gonna do this for consistency
This is a work in progress.
Fixes #417
Notes:
For some reasonFixed. We should not (nor do we need to) overrideembed.getVisitorKeysis being called when using the Svelte plugin — but I'm pretty sure it's supposed to be optional. I don't know why bumping Prettier has caused this to happen. My temporary patch that seems to fix it is to always define it and delegate to printer.getVisitorKeys but that definitely doesn't feel right. Need to figure out why it's being called in the first place.options.printer.Some older versions of some prettier plugins we were testing against aren't compatible with v3.7. Updating them has fixed that issue though.
The current version (v4.0.3) of
prettier-plugin-multiline-arraysis broken with Prettier v3.7+ on its own so those tests are going to fail — not related to our plugin at all:Reproductions for that issue: