feat(rspeedy): express SWC baseline via env instead of jsc.target#2748
Conversation
Move the per-layer SWC compilation baseline from `jsc.target` to `env` (a high `targets` plus an explicit `include` transform list). The emitted build output is byte-identical for existing projects. Since `env` and `jsc.target` are mutually exclusive in SWC, a user-set `tools.swc.jsc.target` now throws a clear error pointing to `tools.swc.env.include`; user-provided `env.include` transforms are merged on top of the baseline (per layer). This unblocks tuning the transform set (e.g. adding `transform-block-scoping`) without changing today's output.
🦋 Changeset detectedLatest commit: c98fa8e The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughMigrates SWC compilation target configuration from ChangesSWC Target Configuration Migration to env
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
@lynx-js/rspeedy
create-rspeedy
@lynx-js/react-rsbuild-plugin
@lynx-js/react-alias-rsbuild-plugin
upgrade-rspeedy
commit: |
Add `afterEach(vi.unstubAllEnvs)` so a stubbed `NODE_ENV` never leaks into a later test. The `include` defaults test relied on an inherited `development` env for the dev-only `@rsbuild/core/dist` include entry; it now stubs that explicitly. Stub the mode only where it is meaningful.
Merging this PR will improve performance by 17.14%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | transform 1000 view elements |
47.1 ms | 40.2 ms | +17.14% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing feat/swc-target-to-env (c98fa8e) with main (87990b5)
Footnotes
-
26 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
- Drop orphaned `config.jsc ??= {}` in swc.plugin.ts (no longer sets jsc.target)
- Trim verbose comment blocks in swc.plugin.ts and loaders.ts
- Rely on the all-dev NODE_ENV leak in swc-config tests (drop afterEach
cleanup and the prod stubs) to match main's existing pattern
No change to emitted build output.
Restore the `toHaveLoader` layer-separation assertions and main's multi-line formatting that were dropped while adapting the layer tests; change only the target-specific assertions that the env refactor forces.
UI JudgeGEQI weighted score: 62.9 / 100 across 8 examples.
DetailsResult 1
Result 2
Result 3
Result 4
Result 5
Result 6
Result 7
Result 8
|
`@lynx-js/genui-a2ui`'s `api-extractor` task runs `build:catalog`, which invokes the genui CLI and dynamically imports `@lynx-js/genui-a2ui-catalog-extractor/dist/cli.js`. The `api-extractor` task only depended on `//#build`, so turbo never guaranteed the catalog extractor was built first — whichever genui package's api-extractor won the (lock-serialized but unordered) race decided pass/fail, intermittently failing CI with "Cannot find module .../a2ui-catalog-extractor/dist/cli.js". Add an explicit `@lynx-js/genui-a2ui-catalog-extractor#build` dependency to genui-a2ui's `api-extractor` task so the CLI is always present.
…tor (#2751) ## Summary `@lynx-js/genui-a2ui`'s `api-extractor` task runs `pnpm run build`, whose `build:catalog` step invokes the `genui` CLI and dynamically imports `@lynx-js/genui-a2ui-catalog-extractor/dist/cli.js`. The shared `api-extractor` task only declared `dependsOn: ["//#build"]`, so turbo never guaranteed `@lynx-js/genui-a2ui-catalog-extractor` was built before `genui-a2ui`'s api-extractor ran. The genui packages run `run-api-extractor.mjs` under a shared lock that serializes them but does **not** order them, so whichever genui package's api-extractor happened to run first decided the outcome. When `genui-a2ui` won the race, its `build:catalog` failed with: ``` Cannot find module '.../packages/genui/a2ui-catalog-extractor/dist/cli.js' imported from .../packages/genui/cli/bin/cli.js ``` This intermittently broke `code-style-check` and `test-api` on unrelated PRs. ## Fix Give `genui-a2ui`'s `api-extractor` task an explicit `@lynx-js/genui-a2ui-catalog-extractor#build` dependency (in addition to the existing `//#build`), so turbo always builds the catalog extractor — producing `dist/cli.js` — before the catalog generation step runs. No dependency cycle: `@lynx-js/genui-a2ui-catalog-extractor` only depends on `typedoc`. ## Failing CI (before this fix) - `code-style-check` on #2748 (after merging main, without this fix): https://github.com/lynx-family/lynx-stack/actions/runs/26581640592/job/78316111246 — fails with `Cannot find module .../a2ui-catalog-extractor/dist/cli.js`. ## Verification that the fix works - `turbo api-extractor --filter=@lynx-js/genui-a2ui --dry-run=json` now reports `@lynx-js/genui-a2ui#api-extractor -> ['//#build', '@lynx-js/genui-a2ui-catalog-extractor#build']` (previously `-> ['//#build']`). - This PR's own `code-style-check` passes: https://github.com/lynx-family/lynx-stack/actions/runs/26584778467/job/78327588583 - Cherry-picked onto #2748 (the branch that was failing), the same `code-style-check` job goes from red to green: https://github.com/lynx-family/lynx-stack/actions/runs/26584934732/job/78328149767 ## Test plan - [x] `code-style-check` passes in CI.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4dd58b8671
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/rspeedy/core/src/plugins/swc.plugin.ts (1)
39-45: ⚡ Quick winOnly
env.includeis honored; otherenvfields are silently dropped.
env.targets,env.exclude, and any other userenvfields are discarded here. This is inconsistent withjsc.target, which now throws — a user who setstools.swc.env.excludeto opt out of a transform would be silently ignored rather than told it's unsupported. Consider rejecting unsupportedenvkeys (mirroring thejsc.targeterror) or clearly documenting that onlyincludeis merged. The MAIN_THREAD override inloaders.tshas the same drop behavior.🤖 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 `@packages/rspeedy/core/src/plugins/swc.plugin.ts` around lines 39 - 45, The current swc plugin code unconditionally replaces config.env with a new object containing only targets and include, dropping any user-provided env properties (e.g., env.exclude) which is inconsistent with jsc.target behavior; update the assignment in swc.plugin.ts (the config.env block that calls getESVersionEnvInclude/getESVersionTarget and uses isProd) to either (a) merge user env fields: create config.env = { ...config.env, targets: ES_ENV_TARGETS, include: [ ...getESVersionEnvInclude(getESVersionTarget(isProd)), ...(config.env?.include ?? []) ] } so existing env.exclude and other keys are preserved, or (b) validate config.env for unsupported keys and throw an error mirroring jsc.target behavior; apply the same fix/validation to the MAIN_THREAD override in loaders.ts so user env fields are not silently dropped.packages/rspeedy/plugin-react/src/loaders.ts (1)
13-44: ⚡ Quick winAvoid re-declaring ES transform baseline lists in plugin-react
packages/rspeedy/plugin-react/src/loaders.tsduplicatesES2019_INCLUDE,ES_ENV_TARGETS(chrome: '120'), and the ES2016→ES2019 transform slice that already exist inpackages/rspeedy/core/src/utils/getESVersionTarget.ts(getESVersionEnvInclude,ES_ENV_TARGETS). If the core canonical list ever changes, these copies can silently drift and break the intended es2019-equivalence/byte-identical output.Consider importing/reusing the shared definitions from the core package (or re-export them from the core entry point—
packages/rspeedy/core/package.jsononly exposes.and./client).🤖 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 `@packages/rspeedy/plugin-react/src/loaders.ts` around lines 13 - 44, The arrays MAIN_THREAD_ENV_INCLUDE, MAIN_THREAD_ENV_TARGETS, and ES2016_TO_ES2019_INCLUDE are duplicate copies of the canonical lists in core (getESVersionEnvInclude / ES_ENV_TARGETS); replace these local declarations by importing the shared definitions from the core module (or have core re-export them and import those exports) so the plugin-react file uses the single source of truth (refer to MAIN_THREAD_ENV_INCLUDE, MAIN_THREAD_ENV_TARGETS, ES2016_TO_ES2019_INCLUDE in this file and getESVersionEnvInclude / ES_ENV_TARGETS in core to locate the matching constants), removing the duplicated arrays here and updating any usage sites to the imported names.
🤖 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.
Nitpick comments:
In `@packages/rspeedy/core/src/plugins/swc.plugin.ts`:
- Around line 39-45: The current swc plugin code unconditionally replaces
config.env with a new object containing only targets and include, dropping any
user-provided env properties (e.g., env.exclude) which is inconsistent with
jsc.target behavior; update the assignment in swc.plugin.ts (the config.env
block that calls getESVersionEnvInclude/getESVersionTarget and uses isProd) to
either (a) merge user env fields: create config.env = { ...config.env, targets:
ES_ENV_TARGETS, include: [
...getESVersionEnvInclude(getESVersionTarget(isProd)), ...(config.env?.include
?? []) ] } so existing env.exclude and other keys are preserved, or (b) validate
config.env for unsupported keys and throw an error mirroring jsc.target
behavior; apply the same fix/validation to the MAIN_THREAD override in
loaders.ts so user env fields are not silently dropped.
In `@packages/rspeedy/plugin-react/src/loaders.ts`:
- Around line 13-44: The arrays MAIN_THREAD_ENV_INCLUDE,
MAIN_THREAD_ENV_TARGETS, and ES2016_TO_ES2019_INCLUDE are duplicate copies of
the canonical lists in core (getESVersionEnvInclude / ES_ENV_TARGETS); replace
these local declarations by importing the shared definitions from the core
module (or have core re-export them and import those exports) so the
plugin-react file uses the single source of truth (refer to
MAIN_THREAD_ENV_INCLUDE, MAIN_THREAD_ENV_TARGETS, ES2016_TO_ES2019_INCLUDE in
this file and getESVersionEnvInclude / ES_ENV_TARGETS in core to locate the
matching constants), removing the duplicated arrays here and updating any usage
sites to the imported names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eacd2ee6-00b6-4dcf-9957-46782e6a31d8
📒 Files selected for processing (7)
.changeset/swc-target-to-env.mdpackages/rspeedy/core/src/plugins/swc.plugin.tspackages/rspeedy/core/src/utils/getESVersionTarget.tspackages/rspeedy/core/test/plugins/swc.plugin.test.tspackages/rspeedy/plugin-react/src/loaders.tspackages/rspeedy/plugin-react/src/swc.tspackages/rspeedy/plugin-react/test/swc-config.test.ts
💤 Files with no reviewable changes (1)
- packages/rspeedy/plugin-react/src/swc.ts
The main thread targets an es2019 engine, so its baseline is a platform constant. Drop the attempt to merge user `tools.swc.env.include` onto the main-thread layer: user transforms now extend only the base/background config, matching the previous `jsc.target` behavior (which the main thread also ignored, hardcoding es2019). This removes the `ES2016_TO_ES2019_INCLUDE` filter, which could silently drop a user-provided es2016~es2019 transform from the main thread.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b215e389f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Add a test that compiles a fixture spanning ES2016~ES2025+ syntax with both `jsc.target` and the `env` config built from `getESVersionEnvInclude`, and asserts byte-identical output. If a future SWC upgrade lowers some syntax at the target that the explicit `include` list omits, the outputs diverge and the test fails — catching the drift automatically. Empirically the current list already reproduces `jsc.target` exactly (es2015 and es2019), including the es2024 `v` regex flag and es2025 regex features, so no transforms needed adding; this just locks the equivalence in.
Summary
Move the per-layer SWC compilation baseline from
jsc.targetto SWCenv(a hightargetsbaseline plus an explicitincludetransform list), across@lynx-js/rspeedy(background/base) and@lynx-js/react-rsbuild-plugin(main thread).The artifact format is completely unchanged. For existing projects the emitted output is byte-for-byte identical —
env: { targets: <high>, include: [...] }runs the exact same set of transforms that the previousjsc.targetlowered. Verified by buildingexamples/*withDEBUG='rspeedy,rsbuild' pnpm buildold-vs-new under a determinism control:main-thread.js,background.js, sourcemaps and the chunk hashes are all identical.Why do this:
env(unlikejsc.target) supportsinclude/exclude, so the transform set can be tuned per layer later (e.g. addingtransform-block-scopingfor the let/const to var perf win) without switching config shape.Behavior changes
tools.swc.jsc.targetis no longer accepted (it is mutually exclusive withenv). Instead of silently dropping it, the build now throws a clear error pointing users totools.swc.env.include.tools.swc.env.includetransforms are merged on top of the baseline (background keeps its es2015 set, main thread stays es2019-equivalent), so users can opt into extra transforms.Tests
swc.plugin.test.ts: env-based defaults (prod es2015 / dev es2019, nojsc.target); userjsc.targetthrows; userenv.includeis merged onto the baseline.swc-config.test.ts: main thread uses es2019env; userjsc.targetis rejected; userenv.includeis merged into both layers.@lynx-js/rspeedy+@lynx-js/react-rsbuild-pluginsuites pass (582 tests).Test plan
turbo build+turbo api-extractorchangeset statusshows@lynx-js/rspeedy+@lynx-js/react-rsbuild-pluginbumping minorSummary by CodeRabbit
Chores
Documentation
Tests