[V3] Plugins can no longer override built-in parsers

[IanVS]

Environments:

• Prettier Version: 3.0.0-alpha.3
• Running Prettier via: Node.js API
• Runtime: Node 16, 18
• Operating System: macOS
• Prettier plugins (if any): @ianvs/prettier-plugin-sort-imports

Steps to reproduce:

I maintain a fork of a popular prettier plugin (https://github.com/trivago/prettier-plugin-sort-imports is the original), and we both have this issue, as does https://github.com/tailwindlabs/prettier-plugin-tailwindcss and any other plugin which adds a preprocess to built-in parsers. They all break in prettier 3.0.

I traced it down to a change in the way that parsers are chosen in resolveParser. Previously, the "last" plugin in the loop which defined a parser for a given name would win. Since custom plugins come at the end of the plugin array, this meant they were able to override built-in parsers. But now, by using a for .. of loop with an early exit, the first plugin defining a parser will be used, giving plugins no chance to override or extend them.

Expected behavior:

Plugins should continue to have a way to extend built-in parsers. Or, if we need to define our own custom parsers, we need a way to determine which parser would have been chosen by prettier, so that users do not need to explicitly map their file types to our custom parsers. But again, ideally the previous behavior would be maintained.

Actual behavior:

Custom plugins cannot extend built-in parsers.

[fisker]

Thanks for the feedback. Let's see if there is a better way to support this use case.

[IanVS]

Thanks. If you're thinking about it, finding a way to support multiple plugins at the same time would be icing on the cake. See tailwindlabs/prettier-plugin-tailwindcss#31 for an example of that issue. (#12807 in this repo as well)

[fisker]

So, previously when we call inferParserByLanguage still loop plugins from first one, and give babel as parser, but actually running overridden one?

prettier/src/utils/infer-parser-by-language.js

Line 3 in 284cdf4

function inferParserByLanguage(language, options) {

[IanVS]

Yes if I'm correct in my understanding, the babel string that's inferred doesn't change, but resolveParser did change in the PR I linked above (https://github.com/prettier/prettier/pull/13268/files#diff-c8358202bc5a3fdb28f467ec76840e377dd714156543353736f19a76c8c1a930).

Previously, it would loop over all the plugins, and set a property on an object, so the last one won.

 for (const name of ownNames(plugin.parsers)) {
    Object.defineProperty(parsers, name, ownDescriptor(plugin.parsers, name));
  }

But now, there's an early return from the loop, so the first one wins:

  for (const { parsers } of plugins) {
    if (parsers && Object.prototype.hasOwnProperty.call(parsers, parser)) {
      return parsers[parser];
    }
  }

[fisker]

I understand why it's broken, a simple fix would be loop from last, or put bultin plugins to the last, but feel so weird it worked like this, I guess the old behavior is unexpected... but it happened works.

[IanVS]

Yes, I agree it's a bit hacky. Would love to know if there's a better way to achieve what we're trying to do, especially if it would mean avoiding conflicts with other plugins. Ideally there would be a pipeline for plugins to modify the AST as needed, rather than a single preprocess that we need to hack in to the parsers.

[fisker]

Yeah, I thought about this before #10803, maybe it's a good opportunity to implement.

[fisker]

Please be patience, need more time to think about it.

[IanVS]

No worries! I know 3.0 is still in alpha, and you're looking for feedback from plugin authors, so I figured I'd bring this up. I think it's a great idea to take a small step back and think about an opportunity to make bigger changes in a major release, rather than just restoring the old, accidentally kind-of-working approach we were relying on. :)

[fisker]

To unblock your migration, I'm going to temporarily restore the behavior of this function until we find a better solution. #13732

[fisker]

Released v3.0.0-alpha.4 with #13732

[fisker]

So, if we change parser infer logic to also loop from the end, your plugins can define a new parser, and Prettier will infer your new parser instead of builtins? 😄

[IanVS]

How can plugins register parser-inferring logic? Is that something that's possible to do today? Would that solve the issue of multiple plugins all using different preprocess functions to change the AST?

[davidaurelio]

Can we please, please get a second category of plugins, e.g. AST Plugins instead of having to override parsers?

Plugins that override parsers seem to be a hack, effectively using a system that was designed to provide additional syntaxes to inject their own logic. This comes with several drawbacks:

• Since plugins are overriding built in parsers, there can only ever be one of them, …
• unless the plugin itself makes sure to call preprocessors of other plugins, like prettier-plugin-multiline-arrays does (finding other plugins, calling them).
• Plugins using preprocess means that source code gets parsed and turned back into a string multiple times. This is bad for speed.
• Plugins import modules from prettier/…, effectively requiring them to be colocated with the prettier version that calls them.
• Plugins import the parsers they know about (like "babel" and "typescript", instead of working for a category of parser, e.g. the ones producing a specific AST format like "estree".

AST-based plugins would get an opportunity to modify the AST of a source file after it has been parsed, and prettier itself would make sure that plugins are called in the configured order without being able to override anything.

That should address the aforementioned drawbacks and offer more flexibility.

I would love to contribute some plugins for logic that is better covered by a code formatter than a linter, but the current way these are implemented is a “very creative use” of Prettier’s plugin API that was meant for something entirely different.

[davidaurelio]

How about this:

• plugins can specify transformers
• these work similarly to printers, i.e. are keyed by AST format.
• they expose a function that works similarly to printer#preprocess, with the difference that Prettier calls all defined transformers for an AST format.
• Prettier 3.0 can then disable the ability to override existing languages, parsers, and printers.
• Maybe this could even be backported to Prettier v2, to become available more quickly.

Would this be a sensible approach? I haven’t really looked at Prettier’s code base. If we can agree on something that works, I might find some time this week to help out.

[fisker]

Thanks for sharing the idea, but I don't think we should encourage plugins mutate AST, since we rely on original text in many places.

I'd rather do multiple "parser-print" process.

[davidaurelio]

@fisker thank you for getting back! Just to aid my understanding:

• is that to guarantee locStart and locEnd keep working, or is there more to it?
• how would a transformer plugin differ from one that uses printers#preprocess? both can manipulate the AST, and the latter already is part of the plugin API. In other words, as a plugin author, I can already manipulate the AST. I should just jump through the hoops and call other preprocessors, too.

Thanks again!

[fisker]

or is there more to it?

Yes, search options.originalText

I can already manipulate the AST

Of course, you can, but that's supposed to use before/after parse or print plugin's own AST, not the bultin ones (or other plugins).

[davidaurelio]

Thanks for taking the time to explain.

Yes, search options.originalText

Gonna do that now.

Of course, you can, but that's supposed to use before/after parse or print plugin's own AST, not the bultin ones (or other plugins).

I would argue the same is true for plugin parsers. It looks as if the idea never was to allow existing ones to be overridden.

Either way, even if AST-based plugins don’t seem feasible, I think the original point still stands: It would be nice to have a plugin system that allows augmenting existing functionality, rather than using the extension hooks in ways they weren’t intended to use.

That way, prettier could have a composable plugin system where plugins can coexist without having to do runtime inspection of other plugins and built-ins.

Look at all the trickery the aforementioned multiline-arrays goes through to make sure its printer is picked. Wouldn’t it be much nicer to have a blessed way to do all this?

[fisker]

Wouldn’t it be much nicer to have a blessed way to do all this?

We never support/expect use Prettier this way, it just working.

[davidaurelio]

Seems I misunderstood the idea behind temporarily re-enabling the old parser finding logic, then.

[fisker]

Let's say if we freeze AST, it won't be a breaking change. We just need find a better way to support run multiple plugins on same file, but until we find a good solution, we have to let Prettier work the old way, so we won't break these plugins.

[IanVS]

Let's say if we freeze AST, it won't be a breaking change.

You may not intend for it to be a breaking change, but it will break a number of plugins which modify the AST that have hundreds of thousands of weekly npm downloads (or more). Since it is apparent that this is a useful hack for prettier plugins, are you certain it's not worth finding a way to pave the cowpath and formally allow such plugins?

[fisker]

We are discussing this design #13729 (comment), I'm just saying it not very good idea, I never said

not worth finding a way

[IanVS]

OK I misunderstood, thanks.

[IanVS]

Hi, just checking in, has there been any more thought into how this can be handled in V3?