fix(js): protect ASI for conditional expressions under experimentalTernaries - #19923
fix(js): protect ASI for conditional expressions under experimentalTernaries#19923koding88 wants to merge 3 commits into
Conversation
✅ Deploy Preview for prettier ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR fixes a JavaScript ASI (automatic semicolon insertion) hazard when experimentalTernaries is enabled and --no-semi is used, ensuring Prettier inserts a leading semicolon before parenthesized multiline ternary conditions so runtime semantics aren’t changed.
Changes:
- Treat
ConditionalExpressionas needing ASI protection whenoptions.experimentalTernariesis enabled. - Add a regression test + snapshot covering the
--no-semi+experimentalTernariesmultiline ternary condition case (issue #18965). - Add an unreleased changelog entry documenting the fix.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/format/js/ternaries/issue-18965/issue-18965.js | Adds a minimal reproduction input for the ASI hazard scenario. |
| tests/format/js/ternaries/issue-18965/format.test.js | Adds a format test enabling experimentalTernaries with semi: false. |
| tests/format/js/ternaries/issue-18965/snapshots/format.test.js.snap | Verifies the output now includes the leading ;( under --no-semi. |
| src/language-js/semicolon/semicolon.js | Ensures ConditionalExpression gets leading semicolon protection under experimentalTernaries. |
| changelog_unreleased/javascript/19923.md | Documents the bug and the corrected output in the unreleased changelog. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
✅ Deploy Preview for prettier ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
fisker
left a comment
There was a problem hiding this comment.
Prettier PR #19923
Playground link
--parser babel
--no-semi
--experimental-ternariesInput:
let o = 1;
a ?
b
: cOutput:
let o = 1
;a ? b : cThe static ConditionalExpression case in expressionNeedsAsiProtection
added a leading semicolon to every ternary expression statement under
experimentalTernaries, even when the test group doesn't break and no
parentheses are printed (e.g. 'a ? b : c' became ';a ? b : c').
Instead, tie the leading semicolon to the same ifBreak as the wrapping
parenthesis around the test in printTernary, so the ';' and '(' always
appear together: break -> ';(...', fit -> no semicolon.
|
The false positive came from handling |
Fixes #18965
Summary
Adds
ConditionalExpressiontoexpressionNeedsAsiProtection()whenexperimentalTernariesis enabled, preventing dangerous semicolon omission before parenthesized multiline ternary conditions under--no-semi.Background
Under
experimentalTernaries, multiline ternary conditions are enclosed in parentheses viawrapInParens(print("test")). When combined with--no-semi, the leading(of a multiline ternary expression statement requires a leading semicolon to prevent unintended function invocation with the preceding statement:Previously,
expressionNeedsAsiProtection()did not handleConditionalExpression, resulting in the omitted semicolon and a runtimeTypeError: 1 is not a function. AddingConditionalExpressionwhenoptions.experimentalTernariesis set ensures the leading semicolon is correctly inserted.Verification
yarn jest tests/format/js/ternaries/issue-18965/format.test.js: pass.yarn test tests/format/js/conditional/ tests/format/js/no-semi/: pass.yarn test: 35,272 tests passed (1,557 suites, 0 regressions).yarn lint:prettier: clean.