perf(test): transpile runtime .ts imports with oxc-node instead of tsx#5965
Conversation
Deploying egg-v3 with
|
| Latest commit: |
b6388bb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://724f6596.egg-v3.pages.dev |
| Branch Preview URL: | https://chore-oxc-node-transpile.egg-v3.pages.dev |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughUpgrades Changes
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)
✨ Finishing Touches🧪 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 updates the test environment configuration in vitest.config.ts to use @oxc-node/core/register instead of tsx/esm for transpiling runtime imports, which improves performance and decorator support. To support this, @oxc-node/core is added to package.json and upgraded to version ^0.1.0 in pnpm-workspace.yaml. There are no review comments, and I have no additional feedback to provide.
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.
Deploying egg with
|
| Latest commit: |
b6388bb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8f377969.egg-cci.pages.dev |
| Branch Preview URL: | https://chore-oxc-node-transpile.egg-cci.pages.dev |
|
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. |
There was a problem hiding this comment.
Pull request overview
This PR speeds up Vitest runs by switching the runtime TypeScript transpilation hook used by child processes from tsx/esm to @oxc-node/core/register, and updates the monorepo’s dependency catalog accordingly.
Changes:
- Update root
vitest.config.tsto setNODE_OPTIONS=--import=@oxc-node/core/register(and replace stale crash-related comments). - Bump
@oxc-node/coreinpnpm-workspace.yamlcatalog to^0.1.0. - Add
@oxc-node/core(catalog-managed) to rootdevDependencies.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| vitest.config.ts | Switch runtime TS preload hook from tsx/esm to @oxc-node/core/register via NODE_OPTIONS. |
| pnpm-workspace.yaml | Bump catalog version for @oxc-node/core to the required range. |
| package.json | Add @oxc-node/core as a root devDependency using catalog:. |
| // fixtures/plugins/app code, and the workspace `src` exports under | ||
| // node_modules) with oxc-node — noticeably faster than tsx and it handles | ||
| // decorators correctly. Requires @oxc-node/core >= 0.1.0, which fixes the | ||
| // earlier "Cannot read properties of undefined (reading 'mode')" crash. | ||
| NODE_OPTIONS: '--import=@oxc-node/core/register', |
There was a problem hiding this comment.
Good catch — fixed in 6c34277. tegg/core/vitest/test/setup.ts now skips injecting a loader when either @oxc-node/core/register or tsx/esm is already present in NODE_OPTIONS (and falls back to oxc-node otherwise), so the two hooks are no longer enabled together. Verified the tegg/core/* project tests still pass.
killagu
force-pushed
the
chore-oxc-node-transpile
branch
from
b3633f6 to
9a32d71
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5965 +/- ##
==========================================
- Coverage 84.92% 82.86% -2.07%
==========================================
Files 676 676
Lines 20513 20522 +9
Branches 4058 4060 +2
==========================================
- Hits 17421 17005 -416
- Misses 2663 3039 +376
- Partials 429 478 +49 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@vitest.config.ts`:
- Line 44: The setup currently appends "--import=tsx/esm" unconditionally when
missing, causing both hooks to be enabled even though vitest.config.ts already
sets NODE_OPTIONS to '--import=`@oxc-node/core/register`'; update the logic that
appends the import (the code that checks for and adds '--import=tsx/esm') to
first inspect NODE_OPTIONS (or the existing import string) for the substring
'`@oxc-node/core/register`' and skip adding '--import=tsx/esm' if that register
import is present; if neither import exists, add '--import=tsx/esm' as before
and ensure you merge into NODE_OPTIONS using the same separator and quoting
rules.
In `@wiki/concepts/vitest-isolate-false-state-leaks.md`:
- Line 53: The markdown list item at line 53 in
vitest-isolate-false-state-leaks.md has broken formatting where the text
`'`@eggjs/`<x>/package.json'` appears on a new unindented line, disrupting the
numbered list flow. Move this backtick-enclosed text back onto the same line as
the preceding text "This single leak caused most of the cross-project" to
restore proper paragraph continuity within the list item, ensuring consistent
indentation and markdown rendering.
In `@wiki/log.md`:
- Line 9: The log note uses the wrong symbol name "mock.mockContext()" which is
inconsistent with the rest of the API; update the text to use the exact symbol
"mockContext()" and verify other mentions in this block (e.g.
`setSnapshotModuleLoader`, `_snapshotModuleLoader`, `isESM`) are spelled exactly
as in code so the documentation matches the real API names.
🪄 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: 79c19fcf-0d06-4019-8f8e-336ff404d3c6
📒 Files selected for processing (9)
package.jsonpackages/utils/src/import.tspackages/utils/test/snapshot-import.test.tsplugins/mock/src/app/extend/application.tspnpm-workspace.yamlvitest.config.tswiki/concepts/vitest-isolate-false-state-leaks.mdwiki/index.mdwiki/log.md
Address PR #5965 review feedback: - tegg/core/vitest/test/setup.ts only appended `--import=tsx/esm` when `tsx/esm` was absent. Now the root vitest config exports `NODE_OPTIONS=--import=@oxc-node/core/register`, so the old guard would enable both loaders at once. Skip injection when either loader is already present and fall back to oxc-node otherwise, keeping a single hook. (Copilot / CodeRabbit) - wiki/log.md: `mock.mockContext()` -> `mockContext()` to match the real API. - wiki/concepts: fix broken numbered-list continuation line. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
killagu
force-pushed
the
chore-oxc-node-transpile
branch
from
9a32d71 to
6c34277
Compare
|
Rebased onto
The PR is now reduced to just the oxc-node migration (
Status is now MERGEABLE. Locally validated the |
The vitest run loads many .ts files at runtime via Node's native `import()` (the egg loader resolving fixtures/plugins/app code, plus workspace `src` exports under node_modules). That path is driven by the `--import` hook in `test.env.NODE_OPTIONS`, previously `tsx/esm`. Switch it to `@oxc-node/core/register` (oxc-based, Rust): it is noticeably faster than tsx and transpiles workspace `src` + decorators correctly. `@oxc-node/core` >= 0.1.0 fixes the earlier "Cannot read properties of undefined (reading 'mode')" crash that blocked this swap (the old FIXME). Full Node-22 suite (isolate:false, --retry 2): same pass/fail result as tsx (only the env-dependent orm/ssrf failures), with the runtime-transpile portion ~6-13% faster depending on how transpile-heavy the workload is. tsx is kept as a dependency — it is still used by worker_threads tests and a few other spots. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR #5965 review feedback: - tegg/core/vitest/test/setup.ts only appended `--import=tsx/esm` when `tsx/esm` was absent. Now the root vitest config exports `NODE_OPTIONS=--import=@oxc-node/core/register`, so the old guard would enable both loaders at once. Skip injection when either loader is already present and fall back to oxc-node otherwise, keeping a single hook. (Copilot / CodeRabbit) - wiki/log.md: `mock.mockContext()` -> `mockContext()` to match the real API. - wiki/concepts: fix broken numbered-list continuation line. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
killagu
force-pushed
the
chore-oxc-node-transpile
branch
from
6c34277 to
2639f00
Compare
|
Rebased again onto the latest On the earlier macos-22
Re-triggered CI on the rebased commit. |
… race When several Egg apps boot concurrently in one process (e.g. tegg multi-app isolation, or vitest `pool:threads` + `isolate:false` running many `mm.app` instances), multiple loaders call `importModule()` on the SAME `.ts` module at the same time. The runtime TypeScript transpile loaders (`tsx`, `@oxc-node/core`) recompile a module on every `import()` — tsx appends a cache-busting query, so Node does not dedupe them — and two simultaneous compiles of the same module can leave one caller observing a partially-initialized namespace whose `default` export is `undefined`. That surfaced nondeterministically as either: - `TypeError: Cannot convert undefined or null to object` in `loadExtend` (`Object.getOwnPropertyNames(undefined)`), or - `Can not find plugin watcher` — a built-in plugin loaded without its `path` because the framework's `config/plugin` module came back empty. Share a single in-flight `import()` per file URL so concurrent first-loads of the same module are serialized onto one compile/evaluation. The map only holds in-flight promises (cleared once settled via `then(clear, clear)`, which also avoids leaving an unhandled rejection on the cleanup chain), so sequential loads are unaffected and there is no cross-test state leak under `isolate:false`. Repro: `tegg/plugin/tegg/test/MultiAppParallel.test.ts` flaked ~12-24% under both transpilers; with this change it passes 40/40 under tsx and oxc-node, and the full suite stays green (527 files / 3430 tests, 0 failures). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add root cause #5 (concurrent first-import returns undefined namespace) to the isolate:false concept page and a log entry for the importModule dedup fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fixed the macOS flake at the rootTracked the intermittent macOS Root cause:
The Fix ( Validation:
This also fixes a latent flake on |
…6007) ## Motivation Every forked `egg-bin` CLI (and every `egg-bin dev/test/cov` child) booted TypeScript through `ts-node/register` + a hardcoded `ts-node/esm` `--loader`. ts-node is a JS-based TS compiler and the deprecated `--loader` hook is slow to spin up — on Windows CI this loader startup dominates the `Test bin` job (tens of seconds **per fork**, ~109 forks). The rest of the repo already moved its runtime `.ts` transpilation to `@oxc-node/core/register` (Rust/oxc) in #5965. This PR brings egg-bin in line. ## What changed - **`baseCommand.ts`** — default `--tscompiler` is now `@oxc-node/core/register`. oxc installs **both** a CJS require hook (via pirates) and an ESM `module.register()` hook from a single `--import`, so: - it is injected as one `--import` for CJS *and* ESM apps (its export is `import`-only, so it is resolved through the package main rather than CJS-resolved / `--require`d); - ESM apps no longer get a separate `ts-node/esm` `--loader`. - `tsconfig-paths/register` is still injected (oxc does not resolve tsconfig `paths`). - **Backward compatible**: legacy compilers keep their exact old path. `--tscompiler=ts-node/register`, `@swc-node/register`, `esbuild-register`, `pkg.egg.tscompiler` and `TS_COMPILER` all still resolve a CJS register + the `ts-node/esm` ESM loader, unchanged. `ts-node` stays 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). - Help summary + the one assertion that pinned `ts-node/register` updated; stale `ts-node/esm` comments refreshed. - Add `@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 pristine `next` (ts-node) and this branch (oxc), then diffed the per-test pass/fail sets: | | total | passed | failed | skipped | |---|---|---|---|---| | `next` (ts-node) | 125 | 86 | 0 | 39 | | this PR (oxc) | 125 | 86 | 0 | 39 | **Regressions: none. Test set: identical. Failures: none.** Decorator-heavy fixtures, ESM/CJS apps, and the explicit `--tscompiler` override tests all still pass. `tsgo` typecheck and `oxlint --type-aware` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Updated TypeScript startup to use import-based runtime registration via `@oxc-node/core/register`, and refreshed `--tscompiler` help text. * Aligned forked TypeScript CLI startup to match the new runtime registration approach. * **Bug Fixes** * Improved consistency across command, snapshot, and test flows for ESM vs CommonJS loader behavior, with clearer snapshot messaging. * **Tests** * Added Vitest coverage for the legacy (non-oxc) `--tscompiler` initialization path with environment isolation. * Updated related help-text/comment expectations. * **Chores** * Updated tooling/template dependencies and refreshed Windows CI concurrency guidance (no behavior change). <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Motivation
The vitest run loads many
.tsfiles at runtime via Node's nativeimport()— the egg loader resolving fixtures / plugins / app code, plus the workspacesrcexports that resolve undernode_modules(e.g.node_modules/egg/src/index.ts). That path is driven by the--importhook intest.env.NODE_OPTIONS, which wastsx/esm.Swap it to
@oxc-node/core/register(oxc / Rust-based): noticeably faster than tsx, and it transpiles workspacesrc+ decorators correctly.@oxc-node/core>=0.1.0fixes the earlierCannot read properties of undefined (reading 'mode')crash that had blocked this (the stale FIXME in the config).Changes
vitest.config.ts:NODE_OPTIONShooktsx/esm→@oxc-node/core/register; replace the stale FIXME comment.pnpm-workspace.yaml: catalog@oxc-node/core^0.0.35→^0.1.0.package.json: add@oxc-node/core: catalog:to root devDependencies.tsxis intentionally kept as a dependency — it is still used by theworker_threadstests, egg-bundler, and a couple of other spots.Test evidence (local, Node 22,
isolate:false,--retry 2)Full suite, oxc-node vs tsx — identical pass/fail (only the env-dependent
orm-plugin/ssrffailures), zero transpile errors across all decorator-heavy tegg / cluster / mock packages:tests(cumulative; where the runtime hook applies)A transpile-heavy subset showed up to ~13%. The win is on the runtime-transpilation portion; overall suite time is dominated by egg app boot + real I/O, so the wall-clock delta is modest but consistent.
Note
Based on
worktree-vitest-isolate-false(the isolate:false fix branch / #5964) so CI runs on a green base and the diff is only this change. Will retarget tonextonce that lands.🤖 Generated with Claude Code
Summary by CodeRabbit
.tsimports during tests to the newer register-based approach, reducing crashes.