fix(es/transforms): strip @flow and @noflow pragma comments#11810
fix(es/transforms): strip @flow and @noflow pragma comments#11810oblador wants to merge 2 commits intoswc-project:mainfrom
@flow and @noflow pragma comments#11810Conversation
…transforming Flow syntax
🦋 Changeset detectedLatest commit: fad0bb3 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Merging this PR will not alter performance
Comparing 
Footnotes
|
There was a problem hiding this comment.
Pull request overview
This PR ensures Flow pragma comments (@flow / @noflow) are removed from emitted JavaScript after Flow type syntax has been stripped, aligning SWC output with Babel’s Flow strip behavior and avoiding Flow re-parsing edge cases.
Changes:
- Add a new
flow_pragma_strippass to remove@flow/@noflowpragma comments while preserving unrelated banner content. - Wire the new pass into SWC’s main transform pipeline for Flow syntax.
- Add targeted tests plus a changeset entry for the behavior change.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
crates/swc_ecma_transforms_typescript/src/flow.rs |
Implements the comment-stripping pass for Flow pragmas. |
crates/swc_ecma_transforms_typescript/src/lib.rs |
Exposes the new pass from the crate API and adds the module. |
crates/swc/src/config/mod.rs |
Enables the pass when syntax.flow() is active. |
crates/swc_ecma_transforms_typescript/tests/flow_pragma_strip.rs |
Adds integration-style tests covering pragma stripping and preservation. |
.changeset/flow-pragma-strip.md |
Records the patch-level change for releases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn starts_with_pragma(line: &str, pragma: &str) -> bool { | ||
| match line.strip_prefix(pragma) { | ||
| Some(rest) => rest.is_empty() || rest.starts_with(|c: char| c.is_whitespace()), | ||
| None => false, | ||
| } | ||
| } |
There was a problem hiding this comment.
starts_with_pragma/line_has_pragma treat any line starting with @flow followed by whitespace as a pragma, which means comments like // @flow is great (or other non-standard modifiers) would be stripped. This is broader than the documented/expected Babel behavior (only @flow, @flow strict, @flow strict-local, @flow weak, and @noflow). Consider validating the remainder so only the supported forms match, and leave other @flow ... text intact.
| &mut recovered_errors, | ||
| ) | ||
| .map_err(|err| err.into_diagnostic(handler).emit())?; | ||
|
|
There was a problem hiding this comment.
recovered_errors from parse_file_as_program are currently ignored. That can make these tests pass even when the parser had recoverable syntax errors. Consider emitting/returning an error (as in tests/flow_strip_correctness.rs) or at least asserting recovered_errors.is_empty() before continuing.
| if !recovered_errors.is_empty() { | |
| for err in recovered_errors { | |
| err.into_diagnostic(handler).emit(); | |
| } | |
| return Err(()); | |
| } |
| swc_ecma_transforms_typescript: patch | ||
| --- | ||
|
|
||
| Strip `@flow` and `@noflow` pragma comments when transforming Flow syntax |
There was a problem hiding this comment.
The changeset summary line doesn’t follow the documented fix(scope): ... style used in this repo’s changeset examples. Consider rewording it to something like fix(es/transforms): strip @flow and @noflow pragma comments when transforming Flow syntax for consistency.
| Strip `@flow` and `@noflow` pragma comments when transforming Flow syntax | |
| fix(es/transforms): strip `@flow` and `@noflow` pragma comments when transforming Flow syntax |
| pub use self::typescript::*; | ||
| pub use self::{flow::flow_pragma_strip, typescript::*}; | ||
| mod config; | ||
| pub mod flow; |
There was a problem hiding this comment.
pub mod flow; makes the entire flow module part of the public API surface. If the intent is only to expose flow_pragma_strip, consider keeping the module private (mod flow;) and re-exporting the function (as you already do) to avoid locking in extra public API.
| pub mod flow; | |
| mod flow; |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6479ffb72
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| fn starts_with_pragma(line: &str, pragma: &str) -> bool { | ||
| match line.strip_prefix(pragma) { | ||
| Some(rest) => rest.is_empty() || rest.starts_with(|c: char| c.is_whitespace()), |
There was a problem hiding this comment.
The new matcher treats any leading line that starts with @flow (followed by any whitespace/text) as a pragma, so comments like // @flow migration notes or /* @flow TODO remove */ are stripped even though they are not one of the documented directive forms (@flow, @flow strict, @flow strict-local, @flow weak, @noflow). This can silently remove user-authored banner/documentation comments in Flow files; the check should validate the full directive suffix instead of only checking a prefix plus whitespace.
Useful? React with 👍 / 👎.
Binary Sizes
Commit: 6027e32 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fad0bb3035
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| Optional::new( | ||
| typescript::flow_pragma_strip::<Option<&dyn Comments>>( | ||
| comments.map(|v| v as _), | ||
| ), | ||
| syntax.flow(), |
There was a problem hiding this comment.
Placing flow_pragma_strip after the TypeScript/Flow stripping pass means it only sees comment anchors that still have corresponding AST nodes; with a shebang, an @flow comment is often attached to the original first statement, and if that statement is removed (for example a Flow import type), the pragma comment remains at the old byte position and is no longer visited by flow_pragma_strip::visit_mut_module/script. In preserve_all_comments mode, dropped_comments_preserver can then reattach that orphaned pragma to surviving nodes, so @flow/@noflow leaks into emitted JS.
Useful? React with 👍 / 👎.
kdy1
left a comment
kdy1
left a comment
There was a problem hiding this comment.
Please remove the new visitor and use another approach. The binary size and the runtime performance will explode
Description: When debugging some other issues I noticed that the flow pragma comments were retained after flow stripping. While technically regular JS can be parsed as flow, it still seems incorrect and I noticed some edge cases where some JS comments caused flow parser to trip up trying to parse them as types. This PR changes this behaviour to strip flow pragma either in its entirety or if multiline only the pragma itself, making it align with the existing babel transform
BREAKING CHANGE: No.
Related issue (if exists): None