Repository navigation
Angular: Serve ancestor node_modules for addon-vitest in browser mode - #35600
Conversation
In an Angular workspace library, `ng generate library` creates `projects/<lib>/package.json` while `node_modules` stays at the workspace root. Vite 7 only treats `pnpm-workspace.yaml`, `lerna.json`, or a `workspaces` field as workspace-root markers (not `.git` or a lockfile), so `searchForWorkspaceRoot` resolves the root to `projects/<lib>`. Vitest's browser server then builds `server.fs.allow` from that subfolder, and the `@angular/compiler` setup file injected by `storybookAngularVitest` — which resolves from the ancestor `node_modules` — falls outside the allow list and fails with "Failed to fetch dynamically imported module". Add every ancestor directory that contains `node_modules` to the browser server's `fs.allow` from `storybookAngularVitest`'s `configureServer`. That hook is used deliberately: Vitest builds the browser server's allow-list itself, so entries added via the `config` hook or static config never reach it. All ancestors are kept (not just the nearest) because Storybook's cache creates a `node_modules/.cache` in the project directory, which would otherwise shadow the real workspace root higher up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FsoQV75vUEGjWo6YFd35cj
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds ChangesAngular Vite filesystem access
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Sequence Diagram(s)sequenceDiagram
participant AngularVitestPlugin
participant findNodeModulesRoots
participant ViteServer
AngularVitestPlugin->>findNodeModulesRoots: discover ancestor node_modules roots
findNodeModulesRoots-->>AngularVitestPlugin: return normalized roots
AngularVitestPlugin->>ViteServer: append roots to server.fs.allow
✨ Finishing Touches📝 Generate docstrings
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.
🧹 Nitpick comments (1)
code/frameworks/angular-vite/src/node/find-node-modules-roots.test.ts (1)
10-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the repository’s spy-enabled, typed Vitest mock pattern.
This mock omits
{ spy: true }, configures mock behavior from test bodies/afterEach, and usesfs as any, which defeats type safety. Align it with the project’sspy-mocking.mdcpattern and move mock setup/reset intobeforeEach.As per coding guidelines, Vitest mocks require
spy: true, behavior inbeforeEach, and type-safe access throughvi.mocked().Also applies to: 21-43
🤖 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 `@code/frameworks/angular-vite/src/node/find-node-modules-roots.test.ts` around lines 10 - 17, Update the node:fs mock and test lifecycle in find-node-modules-roots.test.ts to follow the repository’s spy-enabled Vitest pattern: configure the mock with { spy: true }, move mock setup and reset into beforeEach, and replace the fs as any access with a type-safe vi.mocked() reference. Preserve the existing mocked filesystem behavior and test coverage.Source: Coding guidelines
🤖 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 `@code/frameworks/angular-vite/src/node/find-node-modules-roots.test.ts`:
- Around line 10-17: Update the node:fs mock and test lifecycle in
find-node-modules-roots.test.ts to follow the repository’s spy-enabled Vitest
pattern: configure the mock with { spy: true }, move mock setup and reset into
beforeEach, and replace the fs as any access with a type-safe vi.mocked()
reference. Preserve the existing mocked filesystem behavior and test coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d44c8134-9bfc-4d3c-a5d9-ea58820a18d1
📒 Files selected for processing (2)
code/frameworks/angular-vite/src/node/find-node-modules-roots.test.tscode/frameworks/angular-vite/src/node/vitest.ts
…ots test
Switch to `vi.mock('node:fs', { spy: true })` with the mock behavior set in
`beforeEach` and type-safe `vi.mocked(existsSync)` access, matching the repo's
Vitest mocking guidance and removing the `fs as any` cast.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FsoQV75vUEGjWo6YFd35cj
…les roots Address review feedback: ensure the workspace-root node_modules are allowed even if `server.fs.allow` is ever absent, by initializing it before pushing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FsoQV75vUEGjWo6YFd35cj
LogDetails |
Closes #35578
What I did
Fixes
@storybook/addon-vitestfailing to run story tests in an Angular workspace library (ng generate library), where every suite fails with:Root cause. In an Angular workspace,
ng generate librarycreatesprojects/<lib>/package.jsonwhilenode_modulesstays at the workspace root. Vite 7 only treatspnpm-workspace.yaml,lerna.json, or aworkspacesfield as workspace-root markers (not.gitor a lockfile), sosearchForWorkspaceRootresolves the root toprojects/<lib>. Vitest's browser server then buildsserver.fs.allowfrom that subfolder, and the@angular/compilersetup file injected bystorybookAngularVitest— which resolves from the ancestornode_modules— falls outside the allow list and 404s in the browser.Fix.
storybookAngularVitest()now adds every ancestor directory that containsnode_modulesto the browser server'sfs.allowfrom aconfigureServerhook. Notes on the approach:configureServeris used deliberately: Vitest builds the browser server's allow-list itself, so entries added via theconfighook or staticserver.fs.allownever reach it.node_modules/.cachein the project directory, which would otherwise shadow the real workspace root higher up.normalizePathto match Vite's own allow-list convention on Windows.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
findNodeModulesRootsis covered by unit tests (code/frameworks/angular-vite/src/node/find-node-modules-roots.test.ts), including the case where a nearernode_modules/.cachemust not shadow the real workspace root.Manual testing
ng new my-workspace --no-create-application cd my-workspace ng generate library my-lib npx storybook@latest initprojects/my-lib/vitest.config.tsandprojects/my-lib/.storybook, withnode_modulesat the workspace root.)vitest runagainstprojects/my-lib/vitest.config.ts).Failed to fetch dynamically imported module … @angular/compiler.This was verified end-to-end against exactly this scaffold: baseline reproduces the reported error (3 files failed); with the fix the suites pass (3 files / 8 tests).
Documentation
No documentation changes — this is an internal bug fix.
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsDeclare whether manual QA will be needed for this PR during the next release, through
qa:neededorqa:skipMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit
node_modulesroots, ensuring Angular compiler setup files resolve reliably.node_moduleslocations while selecting the correct workspace-level roots.node_modulesancestor roots and the case where none are found.