perf(egg-bin): boot TypeScript via @oxc-node/core instead of ts-node#6007
Conversation
Replace `ts-node/register` (CJS) + the hardcoded `ts-node/esm` `--loader` with a single `--import @oxc-node/core/register`, the Rust/oxc loader already used by the root test suite (#5965). oxc installs both a CJS require hook and an ESM `module.register()` hook in one shot, so ESM apps no longer need a separate loader. This removes the slow ts-node startup every forked egg-bin CLI paid (tens of seconds on Windows CI). - baseCommand: default `tscompiler` is now `@oxc-node/core/register`; its import-only export is injected via `--import` (resolved through the package main, since it cannot be CJS-resolved). Legacy compilers (ts-node, swc, esbuild) keep their CJS register + ts-node/esm loader path, so explicit `--tscompiler=...` overrides behave exactly as before. tsconfig-paths/register is still injected (oxc does not resolve tsconfig `paths`). - test/coffee.ts: fork the egg-bin CLI via `--import @oxc-node/core/register`. - add `@oxc-node/core` dependency; keep `ts-node` for explicit opt-in. Full egg-bin suite identical before/after (built, CI-style): 86 passed, 0 failed, 39 skipped — zero regressions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
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 (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughReplaces Changesoxc-node/core as default TS compiler
Sequence Diagram(s)sequenceDiagram
participant TestRunner as Test runner
participant BaseCommand as BaseCommand#afterInit
participant Node as Node process
participant OxcRegister as `@oxc-node/core/register`
participant LegacyCompiler as legacy tscompiler
TestRunner->>BaseCommand: start egg-bin with TypeScript enabled
BaseCommand->>Node: set NODE_OPTIONS / tscompiler defaults
alt oxc compiler path
BaseCommand->>OxcRegister: inject register.mjs via --import
else legacy compiler path
BaseCommand->>LegacyCompiler: resolve CJS register entry
BaseCommand->>Node: inject legacy register via formatImportModule()
end
BaseCommand->>Node: add tsconfig-paths/register
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the default TypeScript compiler/loader in egg-bin from ts-node to @oxc-node/core. This change significantly improves startup performance by registering both CommonJS and ES Module hooks via a single --import flag. The review feedback recommends improving the robustness of the compiler detection logic in baseCommand.ts by replacing the broad .includes() check with a more precise string match or prefix check.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // let child process auto require ts-node too | ||
| this.addNodeOptions(this.formatImportModule(tsNodeRegister)); | ||
| flags.tscompiler = flags.tscompiler ?? '@oxc-node/core/register'; | ||
| isOxcCompiler = flags.tscompiler.includes('@oxc-node/core'); |
There was a problem hiding this comment.
Avoid using broad substring checks like 'includes()' on file paths or package specifiers to prevent incorrect matches on similarly named directories or files. Instead, match against exact path segments or precise specifier prefixes.
| isOxcCompiler = flags.tscompiler.includes('@oxc-node/core'); | |
| isOxcCompiler = flags.tscompiler === '@oxc-node/core/register' || flags.tscompiler.startsWith('@oxc-node/core/'); |
References
- When filtering file paths, avoid broad substring checks like 'includes("node_modules")'. Instead, match against exact path segments to prevent incorrect matches on similarly named directories or files.
There was a problem hiding this comment.
Good catch — addressed in a674b3f. The detection now matches the exact specifier or a @oxc-node/core/ subpath prefix instead of a loose includes(), so a similarly named compiler won't be misdetected as oxc. Also added an in-process test covering the explicit non-oxc --tscompiler register branch.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #6007 +/- ##
==========================================
- Coverage 81.99% 81.92% -0.07%
==========================================
Files 676 677 +1
Lines 20522 20639 +117
Branches 4060 4096 +36
==========================================
+ Hits 16826 16908 +82
- Misses 3189 3217 +28
- Partials 507 514 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Dependency limit exceeded — report not shown. This pull request scan exceeded the 10,000-dependency limit applied to this scan, so the results are incomplete and may be inaccurate. To avoid reporting false positives, Socket has not posted a report. Upgrade your plan to raise the dependency limit and get complete reports, or view the partial scan in the dashboard. Socket is always free for open source. If this is a non-commercial open source project, contact us to request a free Team account. |
Deploying egg with
|
| Latest commit: |
a5cd88d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bc4cdc21.egg-cci.pages.dev |
| Branch Preview URL: | https://feat-egg-bin-oxc-loader.egg-cci.pages.dev |
Deploying egg-v3 with
|
| Latest commit: |
a5cd88d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2e4caf1f.egg-v3.pages.dev |
| Branch Preview URL: | https://feat-egg-bin-oxc-loader.egg-v3.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tools/egg-bin/src/baseCommand.ts`:
- Around line 233-240: The default oxc loader resolution in baseCommand.ts is
still using the app’s module resolution path via flags.tscompiler and
cjsResolve(), which allows apps to shadow egg-bin’s bundled `@oxc-node/core`.
Update the loader lookup in the oxc compiler branch so egg-bin resolves its own
bundled `@oxc-node/core` package instead of relying on the app’s install, and keep
the register.mjs injection logic tied to that resolved package root. Verify the
change around isOxcCompiler, cjsResolve('`@oxc-node/core`'), and the default
'`@oxc-node/core/register`' fallback.
🪄 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: 27bbd879-6f2a-4a4c-9223-4d41be56c0c9
📒 Files selected for processing (6)
tools/egg-bin/package.jsontools/egg-bin/src/baseCommand.tstools/egg-bin/src/commands/snapshot.tstools/egg-bin/src/commands/test.tstools/egg-bin/test/coffee.tstools/egg-bin/test/my-egg-bin.test.ts
- Detect oxc by exact specifier / `@oxc-node/core/` prefix instead of a loose `includes()` substring, so a similarly named compiler can't be misdetected (review feedback from gemini-code-assist). - Add an in-process `test --dry-run` case exercising an explicit non-oxc `--tscompiler` (the ts-node/swc/esbuild CJS-register branch), restoring patch coverage on baseCommand.ts now that oxc — not that branch — is the default. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
When `--tscompiler` is not specified, resolve the bundled `@oxc-node/core` from egg-bin's own install instead of the app's `baseDir` first. Otherwise an app pinning an older `@oxc-node/core` (e.g. the tegg template's `^0.0.35`, below the `>=0.1.0` decorator-metadata floor) would shadow the bundled copy and could crash on startup. An explicit `--tscompiler=@oxc-node/core/...` keeps the normal app-first lookup. Also bump the create-egg tegg template to `@oxc-node/core@^0.1.0`. Review feedback from coderabbitai. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The 2-worker cap was set to bound vitest-in-vitest child-process contention under the slow ts-node/esm fork startup. Now that children boot via @oxc-node/core/register, the per-fork loader tax is much lower, so 4 workers fit and shorten the Windows `Test bin` job. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tools/egg-bin/vitest.config.ts`:
- Line 23: The Windows CI worker cap is currently hidden inside a conditional
spread in vitest.config.ts, so the benchmark parser cannot read it and falls
back to availableParallelism. Update scripts/ci-test-benchmark/environment.js to
recognize this machine-readable conditional maxWorkers shape, or refactor the
vitest config around isWindowsCI/maxWorkers so the cap is exposed in a direct
form the parser can extract.
🪄 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: 7e07c2c4-01ad-4416-887c-308ade621558
📒 Files selected for processing (1)
tools/egg-bin/vitest.config.ts
Raising it to 4 oversaturated the 4-vCPU Windows runner: these vitest-in-vitest forks are CPU-bound (each spawns its own vitest), so 4 workers made every case slower and `test.test.ts > should success with some files` timed out at 120s (total ~29m, no speedup over the 2-worker baseline). Revert to 2; oxc already trims the per-fork loader startup, which is the part that was actually loader-bound. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
Motivation
Every forked
egg-binCLI (and everyegg-bin dev/test/covchild) booted TypeScript throughts-node/register+ a hardcodedts-node/esm--loader. ts-node is a JS-based TS compiler and the deprecated--loaderhook is slow to spin up — on Windows CI this loader startup dominates theTest binjob (tens of seconds per fork, ~109 forks).The rest of the repo already moved its runtime
.tstranspilation to@oxc-node/core/register(Rust/oxc) in #5965. This PR brings egg-bin in line.What changed
baseCommand.ts— default--tscompileris now@oxc-node/core/register. oxc installs both a CJS require hook (via pirates) and an ESMmodule.register()hook from a single--import, so:--importfor CJS and ESM apps (its export isimport-only, so it is resolved through the package main rather than CJS-resolved /--required);ts-node/esm--loader.tsconfig-paths/registeris still injected (oxc does not resolve tsconfigpaths).--tscompiler=ts-node/register,@swc-node/register,esbuild-register,pkg.egg.tscompilerandTS_COMPILERall still resolve a CJS register + thets-node/esmESM loader, unchanged.ts-nodestays a dependency for explicit opt-in.test/coffee.ts— the test harness forks the egg-bin CLI itself via--import @oxc-node/core/register(this is the part that paid the Windows tax).ts-node/registerupdated; stalets-node/esmcomments refreshed.@oxc-node/core(catalog:→^0.1.0, decorator-metadata-correct).Test evidence
Ran the full egg-bin suite built, CI-style (
build → vitest run) on both the pristinenext(ts-node) and this branch (oxc), then diffed the per-test pass/fail sets:next(ts-node)Regressions: none. Test set: identical. Failures: none. Decorator-heavy fixtures, ESM/CJS apps, and the explicit
--tscompileroverride tests all still pass.tsgotypecheck andoxlint --type-awareclean.🤖 Generated with Claude Code
Summary by CodeRabbit
@oxc-node/core/register, and refreshed--tscompilerhelp text.--tscompilerinitialization path with environment isolation.