feat: support using rolldown/utils for parsing#537
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds rolldown as an optional peer dependency and refactors detectImportsOxc to lazily load parseSync from rolldown or oxc-parser (with caching). Updates package metadata, workspace catalog, tests, and README to reflect dual-parser support. ChangesRolldown Parser Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
rolldown/utilsrolldown/utils for parsing
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pnpm-workspace.yaml`:
- Line 32: Move the catalog key "rolldown" so it appears immediately after the
"pkg-types" entry to restore alphabetical ordering; edit the list where
"rolldown" currently sits and cut/paste it to follow the "pkg-types" key,
preserving the existing version string ("^1.0.0") and indentation/formatting.
In `@src/detect-oxc.ts`:
- Around line 1-2: The import order is wrong — alphabetically sort the top
imports so the `estree` import comes before `magic-string`; specifically swap or
reorder the two import statements that bring in `Program` (from 'estree') and
`MagicString` (from 'magic-string') so the `import type { Program } from
'estree'` line appears before `import type MagicString from 'magic-string'`.
In `@test/detect-estree.test.ts`:
- Around line 21-23: Duplicate test suite name: change the second call to
testWith('oxc', code => parseWithRolldownOxc(...).program as Program) to use a
distinct name (e.g., testWith('oxc-module', ...) or similar) so the two suites
are unique; update the string literal passed to testWith while leaving the
callback and parseWithRolldownOxc invocation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 44151a1d-6889-4080-bb74-bde2d650b88f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
README.mdpackage.jsonpnpm-workspace.yamlsrc/detect-oxc.tstest/detect-estree.test.ts
| testWith('oxc', code => parseWithRolldownOxc('', code, { | ||
| sourceType: 'module', | ||
| }).program as Program) |
There was a problem hiding this comment.
Fix duplicate test suite name.
Both line 17 and line 21 call testWith('oxc', ...), creating duplicate test suite names detect-oxc. The second test should use a distinct name to avoid confusion and potential test framework issues.
🧪 Proposed fix
-testWith('oxc', code => parseWithRolldownOxc('', code, {
+testWith('rolldown', code => parseWithRolldownOxc('', code, {
sourceType: 'module',
}).program as Program)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| testWith('oxc', code => parseWithRolldownOxc('', code, { | |
| sourceType: 'module', | |
| }).program as Program) | |
| testWith('rolldown', code => parseWithRolldownOxc('', code, { | |
| sourceType: 'module', | |
| }).program as Program) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/detect-estree.test.ts` around lines 21 - 23, Duplicate test suite name:
change the second call to testWith('oxc', code =>
parseWithRolldownOxc(...).program as Program) to use a distinct name (e.g.,
testWith('oxc-module', ...) or similar) so the two suites are unique; update the
string literal passed to testWith while leaving the callback and
parseWithRolldownOxc invocation unchanged.
|
@copilot would you please reorder the imports so the lint passes 🙏❤️ |
Agent-Logs-Url: https://github.com/unjs/unimport/sessions/c34b71d6-1326-4748-aa01-f64e78f5463a Co-authored-by: danielroe <28706372+danielroe@users.noreply.github.com>
sparked by nuxt/nuxt#34983
this allows using
parseSyncfromrolldown/utils(if installed) rather thanoxc-parser. the aim is to avoid duplicate installs (rolldown bundles oxc-parser).Summary by CodeRabbit
New Features
rolldownas an alternative parser; the system will detect and preferrolldownwhen available and fall back tooxc-parser.Documentation
rolldownas the preferred (optional) peer and to clarify the “or” choice foroxc-parser.Chores
rolldownas an optional peer and pin its workspace entry.