Skip to content
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

feat: support prettier v3 #2100

Merged
merged 5 commits into from
Jul 19, 2023
Merged

feat: support prettier v3 #2100

merged 5 commits into from
Jul 19, 2023

Conversation

dummdidumm
Copy link
Member

@dummdidumm dummdidumm commented Jul 17, 2023

Prettier v3 changed some APIs to be asynchronous. This is an easy fix, we just add await to some methods which is useless but harmless for v2 invocations.

The more involved problem is that the v3 Prettier API is incompatible with v2, and likewise you can't use prettier plugins that support v2 with Prettier v3 and vice versa. This means we need to add additional code to keep things working when someone has Prettier (or the plugin) installed but not the other (the plugin or Prettier). The current code does ignore the user's Prettier version if it's v3 but prettier-plugin-svelte is not found - it falls back to v2 of both Prettier and the plugin. It falls back to v2 for now because based on installs Prettier v2 is still far more popular so that's the safer choice.
There's one problem though: What if someone has other plugins in their node modules but not the prettier plugin? In this case formatting could be done with Prettier v2 using plugins (from the user) that only support v3, and that fails. I don't see a good way around this, but I would also hope this does not happen often.

Thoughts @jasonlyu123 ?

@ghostdevv
Copy link
Member

Perhaps we could have a "best guess" at prettier version and have a way to override it if it guesses wrong?

@jasonlyu123
Copy link
Member

I also can't think of ways around it. Probably easier to just show a message alert. But the problem is, can we reliably identify if the problem is caused by a plugin incompatible with a prettier version? Or we can only provide a troubleshooting hint.

@dummdidumm
Copy link
Member Author

dummdidumm commented Jul 18, 2023

I fear we can't identify if the problem is caused by a plugin, so showing a hint everytime the formatting goes wrong could only be very generic. It would also be shown if everything is ok on the plugin-side but someone just has invalid code, which would be a lot of false positives. I'm therefore against a hint, but we should already log something to the output tab so if people look there they should get a lead.

@dummdidumm
Copy link
Member Author

tested:

  • prettier v3 + plugin v3 -> works
  • prettier v2 + plugin v2 -> works
  • prettier v2 + no plugin -> works
  • prettier v3 + no plugin -> works

Should be safe. I needed to adjust the code some more because Prettier v3 has removed the plugin search feature and so we need to load the config to see if plugins: ['prettier-plugin-svelte'] is in there. Another review would be appreciated.

(will look into if we can test this in the test suite)

@dummdidumm dummdidumm marked this pull request as ready for review July 19, 2023 09:57
@dummdidumm dummdidumm merged commit f8d7c20 into master Jul 19, 2023
2 checks passed
@dummdidumm dummdidumm deleted the prettier-v3-support branch July 19, 2023 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants