-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Remove support for Flow comment types #13687
Conversation
Something isn't right with Prettier pr-13687 --parser babel-flow Input: const c = (data/*: /* this is an object *-/ Object */) => {}; Output: const c = (data: bject) => {}; |
AST shows identifier "bject" |
f9b3069
to
470a598
Compare
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) |
Just don't use the |
Make sense. |
|
470a598
to
17a1d3a
Compare
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 --parser babel Input: const a = (b /*: c */) Output: const a = b /*: c */; |
An idea for a heuristic: if at least one such comment is found in the input (and |
On the other hand, if we decided to "remove support", it should mean it's okay to break that case. 🤔 |
How |
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 |
@gkz What do you think about this change? |
Removing support for this feature from prettier seems reasonable to me.
It roughly is, a lot of our infra already doesn't support it super well (e.g. Flows own
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. |
Will this |
Probably, but not more than it's already possible for the |
Found an inconsistency between Prettier pr-13687 --parser flow Input: (a /* b */ /*: c */) Output: (a: /* b */ c); Prettier pr-13687 --parser babel-flow Output: (a /* b */: c); |
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. |
@@ -0,0 +1,27 @@ | |||
#### Remove support for Flow comments (#13687 by @thorn0) |
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.
@thorn0 This should be marked as [BREAKING]
?
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, let's do that.
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.
Done in #13703
## [1.0.3](v1.0.2...v1.0.3) (2023-09-05) ### Dependencies and Other Build Updates * **deps-dev:** bump @commitlint/cli from 17.6.5 to 17.6.6 ([#53](#53)) ([22b8bbe](22b8bbe)) * **deps-dev:** bump @commitlint/cli from 17.6.6 to 17.6.7 ([#59](#59)) ([702df77](702df77)) * **deps-dev:** bump @commitlint/cli from 17.6.7 to 17.7.0 ([#61](#61)) ([043c953](043c953)) * **deps-dev:** bump @commitlint/cli from 17.7.0 to 17.7.1 ([#63](#63)) ([e58c315](e58c315)) * **deps-dev:** bump @commitlint/config-conventional from 17.6.5 to 17.6.6 ([#54](#54)) ([1d99376](1d99376)) * **deps-dev:** bump @commitlint/config-conventional from 17.6.6 to 17.6.7 ([#58](#58)) ([8a99190](8a99190)) * **deps-dev:** bump @commitlint/config-conventional from 17.6.7 to 17.7.0 ([#62](#62)) ([b9e2f9f](b9e2f9f)) * **deps-dev:** bump prettier from 2.8.8 to 3.0.0 ([#57](#57)) ([2bdc7df](2bdc7df)), closes [prettier/prettier#14435](prettier/prettier#14435) [prettier/prettier#14212](prettier/prettier#14212) [prettier/prettier#14391](prettier/prettier#14391) [prettier/prettier#13687](prettier/prettier#13687) [prettier/prettier#13732](prettier/prettier#13732) [prettier/prettier#13731](prettier/prettier#13731) [#15011](https://github.com/Th3S4mur41/demo-auto-security-release/issues/15011) [#15010](https://github.com/Th3S4mur41/demo-auto-security-release/issues/15010) [#15009](https://github.com/Th3S4mur41/demo-auto-security-release/issues/15009) [#14901](https://github.com/Th3S4mur41/demo-auto-security-release/issues/14901) [#14896](https://github.com/Th3S4mur41/demo-auto-security-release/issues/14896) [#14994](https://github.com/Th3S4mur41/demo-auto-security-release/issues/14994) [#14995](https://github.com/Th3S4mur41/demo-auto-security-release/issues/14995) [#15002](https://github.com/Th3S4mur41/demo-auto-security-release/issues/15002) [#15000](https://github.com/Th3S4mur41/demo-auto-security-release/issues/15000) * **deps-dev:** bump prettier from 3.0.0 to 3.0.1 ([#60](#60)) ([724d609](724d609)) * **deps-dev:** bump prettier from 3.0.1 to 3.0.2 ([#64](#64)) ([9783d95](9783d95)) * **deps-dev:** bump prettier from 3.0.2 to 3.0.3 ([#67](#67)) ([c9a7f2a](c9a7f2a)) * **deps-dev:** bump semantic-release from 21.0.5 to 21.0.6 ([#55](#55)) ([ff8e146](ff8e146)) * **deps-dev:** bump semantic-release from 21.0.6 to 21.0.7 ([#56](#56)) ([b601697](b601697)) * **deps-dev:** bump semantic-release from 21.0.7 to 21.0.9 ([#65](#65)) ([72e37ac](72e37ac)) * **deps-dev:** bump semantic-release from 21.0.9 to 21.1.1 ([#66](#66)) ([d3d00f0](d3d00f0)) * **deps:** bump actions/checkout from 3 to 4 ([#68](#68)) ([14f006f](14f006f)), closes [actions/checkout#1436](actions/checkout#1436) [actions/checkout#1067](actions/checkout#1067) [actions/checkout#1447](actions/checkout#1447) [actions/checkout#1436](actions/checkout#1436) [actions/checkout#1067](actions/checkout#1067) [actions/checkout#1377](actions/checkout#1377) [actions/checkout#579](actions/checkout#579) [actions/checkout#1437](actions/checkout#1437) [actions/checkout#579](actions/checkout#579) [actions/checkout#1437](actions/checkout#1437) [actions/checkout#1196](actions/checkout#1196) [actions/checkout#1287](actions/checkout#1287) [actions/checkout#1369](actions/checkout#1369) [actions/checkout#1376](actions/checkout#1376) [actions/checkout#1196](actions/checkout#1196) [actions/checkout#1287](actions/checkout#1287) [actions/checkout#1369](actions/checkout#1369) [actions/checkout#1289](actions/checkout#1289) [#1286](https://github.com/Th3S4mur41/demo-auto-security-release/issues/1286) [actions/checkout#1246](actions/checkout#1246) [actions/checkout#1246](actions/checkout#1246) [#770](https://github.com/Th3S4mur41/demo-auto-security-release/issues/770) [actions/checkout#1057](actions/checkout#1057) [#1447](https://github.com/Th3S4mur41/demo-auto-security-release/issues/1447) [#1067](https://github.com/Th3S4mur41/demo-auto-security-release/issues/1067) [#1436](https://github.com/Th3S4mur41/demo-auto-security-release/issues/1436)
Description
See #13142 (comment)
Checklist
docs/
directory).changelog_unreleased/*/XXXX.md
file followingchangelog_unreleased/TEMPLATE.md
.✨Try the playground for this PR✨