fix: handle TsTypeAssertion that conflicts with JSX handling for tarball bundling#7049
fix: handle TsTypeAssertion that conflicts with JSX handling for tarball bundling#7049pieh wants to merge 3 commits into
Conversation
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR refactors AST parsing in Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| // We want to make sure we can handle this case and still rewrite the import assertions correctly. | ||
| const source = ` | ||
| import data3 from './data.json' assert { type: 'json' }; | ||
| const params = <Params> inputs; |
There was a problem hiding this comment.
This is valid and documented syntax - https://www.typescriptlang.org/docs/handbook/2/everyday-types.html#type-assertions - it just conflicts in JSX mode because angled brackets become ambiguous so parsers intentionally disable handling of this syntax for type casting in JSX mode
…rtion-when-rewriting-import-assertions
| const parseAST = (source: string): Program => { | ||
| try { | ||
| return acornJSX.parse(source, parseOptions) | ||
| } catch (error) { | ||
| // for non-jsx typescript casting to type via "<type> value" (normally done with "value as type") will throw an "Unexpected token" error in acorn-jsx, | ||
| // but is valid syntax in TypeScript. In this case, we can retry parsing with the non-jsx parser. | ||
| if (error instanceof SyntaxError) { | ||
| return acornNoJSX.parse(source, parseOptions) | ||
| } | ||
| throw error | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential alternative I considered is picking parser based on extension (with .ts specifically using NoJSX mode), but fallback felt like more sure thing that is potentially not susceptible to wild and unexpected configurations
1 participant
🎉 Thanks for submitting a pull request! 🎉
Summary
Fixes #<replace_with_issue_number>
For us to review and ship your PR efficiently, please perform the following steps:
we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
something that`s on fire 🔥 (e.g. incident related), you can skip this step.
your code follows our style guide and passes our tests.
A picture of a cute animal (not mandatory, but encouraged)