Use local prettier instance for parsers#706
Conversation
CiGit
left a comment
There was a problem hiding this comment.
I don't think this is the right fix for this exact problem. IMO the fix lays inside getSupportInfo (on prettier's side)
BUT
I like it.
This is a first step toward plugin support :-)
and it fixes a current problem
| function getSupportLanguages(version?: string) { | ||
| return (require('prettier') as Prettier).getSupportInfo(version).languages; | ||
| function getSupportLanguages(prettierInstance: Prettier = bundledPrettier) { | ||
| return prettierInstance.getSupportInfo(prettierInstance.version).languages; |
There was a problem hiding this comment.
I can't remember when prettier.getSupportInfo was added to prettier. But on quite old prettier versions, this may break.
There was a problem hiding this comment.
Looks like getSupportInfo was added in prettier/prettier#3033 which only landed in 1.8.0. Think it's worth adding a fallback to bundledPrettier.getSupportInfo if getSupportInfo isn't defined?
There was a problem hiding this comment.
It still would resolve to "babel" instead of "babylon". Without a fix in getSupportInfo. So quite pointless right now.
1.8 is 1 year old. Let's expect no one is using it with this extension 😆
Monitoring issues ...
Solution could be to simply use bundled if local version is below a given version. vscode now allows us to easily downgrade an extension. I think we would need to do that to support plugins anyway without having to support too many code path and keep this extension simple.
There was a problem hiding this comment.
Without a fix in getSupportInfo. So quite pointless right now.
Yeah, it'll only be useful once there is an upstream fix in getSupportInfo, but required if that does happen. 🤷♂️
1.8 is 1 year old. Let's expect no one is using it with this extension 😆
You have more faith than I do in people updating deps! 😂 But if you don't think it'll be an issue, works for me 👍
There was a problem hiding this comment.
You have more faith than I do in people updating deps! joy But if you don't think it'll be an issue, works for me +1
Prettier was new and it's not a major version change. That's why I'm expecting so. and certainly mistaken. Let's push people to upgrade their deps 😃
|
Thank you for your contribution! |
Yeah, I think you are right here. But it seemed like a reasonable forward thinking workaround 😂. Hopefully |
|
@briandk Too late. Already released. But it would have been a good idea. We should have a PR template! With a quick glance at getSupportInfo, it would need some changes... It only supports filtering languages not parsers. |
Looks like the version in the repo hasn't been updated to 1.8.1. Should it get the bump? |
|
Looks like I forgot to push ... |
Resolves #705
As part of 1.16.0 of Prettier, the parsers for each version were swapped from
babylontobabel. However this causes the default parser to be used for any local version of Prettier to bebabel, even ifbabelisn't a valid parser for that version.This switches the dynamicParser interpretation to use the local version of Prettier, which will properly include
babyloninstead ofbabel, allowing the extension to run properly on local Prettier instances < 1.16.0 and avoid throwingCouldn't resolve parser "babel"First time contributor, so please let me know if an alternative approach would be preferred, just hoping to get this fixed ASAP. (and avoid migrating a few hundred repos this week 😬).