Remove support for Flow comment types

[thorn0]

Description

See #13142 (comment)

• Issues to check and close: https://github.com/prettier/prettier/issues?q=is%3Aopen+is%3Aissue+label%3A%22area%3Aflow+comment+types%22

Checklist

• I’ve added tests to confirm my change works.
• (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
• (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
• I’ve read the contributing guidelines.

✨Try the playground for this PR✨

[thorn0]

Something isn't right with babel-flow:

Prettier pr-13687
Playground link

--parser babel-flow

Input:

const c = (data/*: /* this is an object *-/ Object */) => {};

Output:

const c = (data: bject) => {};

[fisker]

AST shows identifier "bject"

[thorn0]

Yes, it's a bug in Babel.

https://astexplorer.net/#/gist/5fdbbaf16abcc351777e4f63b57b2e77/c87709ee4645b1d55d9eb27c83077698ffff7a9d

babel/babel#15061

[fisker]

I thought "removing support for Flow comment types" means treat them as normal comments.

Aren't we breaking code? (Making runnable js code not runnable)

[thorn0]

Just don't use the flow and babel-flow parsers if you want to treat them as normal comments.

[fisker]

Make sense.

[fisker]

@babel/parser updated #13654

[thorn0]

Treating them like normal comments will break things for people who use this syntax though. So I don't know how to proceed.

Prettier pr-13687
Playground link

--parser babel

Input:

const a = (b /*: c */)

Output:

const a = b /*: c */;

[thorn0]

An idea for a heuristic: if at least one such comment is found in the input (and parser is flow/babel-flow), print all non-JS constructions as Flow comments.

[thorn0]

On the other hand, if we decided to "remove support", it should mean it's okay to break that case. 🤔

[fisker]

How hermes-eslint handle this？

[thorn0]

I know little about Hermes. But the version of it found on AST explorer doesn't support this comment syntax at all: https://astexplorer.net/#/gist/2fe013d7b342a4a087b9a96394b04971/13ef1d3e2afe4088a56859c37faaccb96c7b7a46
Which might mean this syntax is considered kind of deprecated.

[fisker]

@gkz What do you think about this change?

[pieterv]

Removing support for this feature from prettier seems reasonable to me.

Which might mean this syntax is considered kind of deprecated.

It roughly is, a lot of our infra already doesn't support it super well (e.g. Flows own flow codemod feature) and it's not so useful now that build steps are so ubiquitous even for node scripts. We will likely officially deprecate it from Flow soon.

How hermes-eslint handle this?

hermes-eslint uses the hermes parser (hermes-parser). This is a WASM compiled version of the parser the HermesVM uses. This parser does not have any special support for Flow comments, so they stay comments (just like if you used any other parser). So they still kinda work depending on the accuracy of the prettier comment attachment logic for each comment location.

As a side note this parser works well with prettier, we use it for writing codemods: https://github.com/facebook/hermes/blob/main/tools/hermes-parser/js/hermes-transform/src/transform/transform.js#L66-L76.

[fisker]

Will this /*: break our whitespace/new line detect?

[thorn0]

Probably, but not more than it's already possible for the flow parser.

[thorn0]

Found an inconsistency between flow and babel-flow (example from #7510):

Prettier pr-13687
Playground link

--parser flow

Input:

(a /* b */ /*: c */)

Output:

(a: /* b */ c);

Prettier pr-13687
Playground link

--parser babel-flow

Output:

(a /* b */: c);

[fisker]

Are you going to fix it? Feel it will be difficult. How about just pretend we don't know? 😄

[thorn0]

Are you going to fix it? Feel it will be difficult. How about just pretend we don't know? 😄

I'm curious what's going on there, but I'll open a separate issue for it.

[sosukesuzuki]

@thorn0 This should be marked as [BREAKING]?

[thorn0]

Yes, let's do that.

[thorn0]

Done in #13703