fix(storybook-angular): use oxc config instead of esbuild for Vite 8#2313
Conversation
Backports the esbuild-to-oxc detection from alpha to beta. Vite 8 deprecated the `esbuild` config option in favor of `oxc`, causing deprecation warnings when running Storybook builds. Closes #2312 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for analog-blog ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for analog-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for analog-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe Vite preset for Storybook Angular was updated to replace the esbuild-specific transform plugin with a generic transform-config plugin and to select the transform backend dynamically: it uses Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Observations
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/storybook-angular/src/lib/preset.ts`:
- Line 4: Add unit tests for storybookTransformConfigPlugin's Vite version
detection by mocking vite.rolldownVersion truthy and falsy and asserting the
produced transform config contains the correct key; when vite.rolldownVersion is
truthy expect the plugin to produce { oxc: { keepNames: true } } and when falsy
expect { esbuild: { keepNames: true } }. In your test file import the plugin
factory from packages/storybook-angular/src/lib/preset.ts
(storybookTransformConfigPlugin), stub or spy on vite.rolldownVersion to return
true/false for each test case, invoke the plugin to get its transform config,
and assert the presence of the appropriate key and value.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 634f955d-c40b-4be0-bae6-b1b05220b6df
📒 Files selected for processing (1)
packages/storybook-angular/src/lib/preset.ts
The vite namespace mock needs rolldownVersion defined so the storybookTransformConfigPlugin can access it without vitest throwing a missing-export error. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/storybook-angular/src/lib/preset.spec.ts (1)
24-70: Consider adding coverage for theoxc(Vite 8+) branch.Both mocks pin
rolldownVersion: undefined, which only exercises theesbuildfallback instorybookTransformConfigPlugin. Since this PR's whole reason for existing is the Vite 8 / rolldown path, a tiny companion test that mocksrolldownVersion: '1.0.0'and asserts the resulting plugin emits anoxc: { keepNames: true }config (rather thanesbuild) would lock in the fix and guard against a silent regression if the conditional ever flips. Otherwise the backport is only verified by the branch that already worked.🧪 Sketch
it('uses oxc transform config when rolldownVersion is present (Vite 8+)', async () => { vi.resetModules(); vi.doMock('vite', () => ({ mergeConfig: (_b: unknown, o: unknown) => o, normalizePath: (p: string) => p, rolldownVersion: '1.0.0', })); registerDependencyMocks(); // or inline the other doMocks const { viteFinal: fresh } = await import('./preset'); const result = await fresh(baseConfig, makeOptions({})); const transformPlugin = result.plugins .flat() .find((p: any) => p?.name === 'analogjs-storybook-transform-config'); const cfg = transformPlugin?.config?.(); expect(cfg?.oxc?.keepNames).toBe(true); expect(cfg?.esbuild).toBeUndefined(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storybook-angular/src/lib/preset.spec.ts` around lines 24 - 70, Add a test that exercises the Vite 8+ branch by mocking vite.rolldownVersion to a non-undefined value (e.g., '1.0.0') before re-importing the module and calling viteFinal; reuse registerDependencyMocks to re-register other mocks, then find the plugin named 'analogjs-storybook-transform-config' in the returned config and assert that its config() returns an oxc object with keepNames: true and that esbuild is undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/storybook-angular/src/lib/preset.spec.ts`:
- Around line 24-70: Add a test that exercises the Vite 8+ branch by mocking
vite.rolldownVersion to a non-undefined value (e.g., '1.0.0') before
re-importing the module and calling viteFinal; reuse registerDependencyMocks to
re-register other mocks, then find the plugin named
'analogjs-storybook-transform-config' in the returned config and assert that its
config() returns an oxc object with keepNames: true and that esbuild is
undefined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6944515a-dca8-44ed-98e8-97ef3ad1ed03
📒 Files selected for processing (1)
packages/storybook-angular/src/lib/preset.spec.ts
PR Checklist
Closes #2312
Affected scope
Recommended merge strategy for maintainer [optional]
What is the new behavior?
Backports the esbuild-to-oxc detection from
alphatobeta. Vite 8 deprecated theesbuildconfig option in favor ofoxc, causing deprecation warnings when running Storybook builds.The plugin now checks
vite.rolldownVersion(present in Vite 8+) to decide whether to setoxc: { keepNames: true }oresbuild: { keepNames: true }, matching the pattern already used across the rest of the codebase. Also renames the plugin fromanalogjs-storybook-esbuild-configtoanalogjs-storybook-transform-configto reflect the change.Test plan
nx format:checkpnpm buildpnpm testDoes this PR introduce a breaking change?
Other information
This is a minimal backport of the relevant changes from analogjs/analog#2121 (already on
alpha). The full alpha diff includes unrelated type annotations, debug logging, and SCSS resolution changes that are not included here.🤖 Generated with Claude Code