Skip to content

Angular: Introduce TestBed-based story rendering in @storybook/angular-vite#35119

Draft
valentinpalkovic wants to merge 100 commits into
nextfrom
valentin/angular-vite-testbed
Draft

Angular: Introduce TestBed-based story rendering in @storybook/angular-vite#35119
valentinpalkovic wants to merge 100 commits into
nextfrom
valentin/angular-vite-testbed

Conversation

@valentinpalkovic

@valentinpalkovic valentinpalkovic commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Closes #31311
Targets / stacks on #34202

What I did

This PR finalizes the TestBed rendering work from #31311 (@christoph-rogalla) by re-architecting it on top of the new @storybook/angular-vite package from #34202 (@brandonroberts). It contains all commits from #34202 plus a merge of current next (conflicts resolved), so the diff will collapse once #34202 lands.

TestBed rendering strategy (previewTestBedRenderer feature flag, off by default)

An architecture review of the original PR (with Angular-runtime-level verification) found four critical defects in the original builder approach: change detection was dead after the initial render (play functions interacted with a frozen UI), props binding was broken for non-input props and all outputs, per-render new TestBed() bypassed the singleton contract and leaked fixtures, and the host element was abandoned by transplanting child nodes (breaking :host styles and host listeners). Instead of porting those, the strategy was rebuilt:

  • Single mounting seam: AbstractRenderer gains a protected mountApplication() (default: bootstrapApplication). TestBedRenderer overrides only that seam via mountWithTestBed(). Story preparation, the wrapper component (input/output bindings, storyProps$ updates), and the re-render fast path are all shared — eliminating the original defects by construction.
  • Documented TestBed singleton lifecycle: getTestBed(), initTestEnvironment only when no platform exists (reuses a Vitest-initialized environment), resetTestingModule() per render (destroys the previous fixture incl. ngOnDestroy).
  • Custom TestComponentRenderer mounts the real host element inside the story target node, so :host styles, host listeners and selector semantics keep working.
  • fixture.autoDetectChanges() + await fixture.whenStable() keep change detection alive for play functions.
  • Router support: router.initialNavigation() is triggered manually when a Router is provided (TestBed never "bootstraps", so provideRouter's bootstrap listener doesn't fire).
  • Scope: canvas-only. Docs pages render multiple stories with different application configs which cannot share the single TestBed environment injector, and addon-vitest browser mode owns the TestBed lifecycle itself — both keep using bootstrapApplication.

Fixes for the previously failing stories

All stories from the prior test report are now verified green locally (dev preview, prod build, vitest browser mode):

  • dom-sanitizer-test, router component errors: caused by type-only DI imports (import type { DomSanitizer/Router }) which Vite's type stripping erases, breaking JIT DI → switched to inject(); router story rewritten to standard provideRouter(routes, withHashLocation()) instead of the custom applicationConfig.routing API from the original PR.
  • provider-with-module-wrapper component error: standalone wrapper components cannot import ModuleWithProviders. PropertyExtractor now hoists their providers via importProvidersFrom() and imports the plain NgModule — fixing this for both mounting strategies (replaces the old console warning).
  • Interaction failures (Callback, Sync/Async Wait For, With Loaders, User Event Setup, hooks: Use State, order-of-hooks, rendering: Change Args, Page: Logged In): all consequences of the original dead-change-detection/props-binding defects; fixed by the wrapper-sharing architecture above.

Additional fixes surfaced while testing

  • storybook build failed with File name too long (os error 63): analogjs's JIT support emits inline-style virtual modules whose id embeds the whole base64 stylesheet, and the bundler derives chunk file names from it. Chunk names are now trimmed via an outputOptions plugin hook (survives rollupOptions/rolldownOptions config normalization across vite versions).
  • template/stories_angular-cli-default-ts was never linked into the vite sandbox (sandbox story dirs are keyed by template id) — renamed to stories_angular-cli-vite-default-ts so the signal/model-signal stories actually run; removed the dead stories_angular-cli-prerelease copy.
  • @angular/router added as optional peer dependency; @angular/build added to devDependencies (declared peer, resolved from this package's context by analogjs).
  • clearRootHTMLElement null guards for DOMs without storybook root elements.

The angular-cli/vite-default-ts sandbox enables previewTestBedRenderer: true (last, independently revertible commit), so CI exercises the TestBed strategy for all canvas template stories. Classic @storybook/angular is untouched.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

  1. Run a sandbox for template angular-cli/vite-default-ts
  2. Open Storybook in your browser
  3. Verify the previously failing stories (sanitizer, router, providers, hooks, rendering, component-test basics) pass, in story view, docs view and the built storybook

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts
  • Make sure this PR contains one of the labels below: feature request

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features

    • Added Angular Vite framework support for Storybook with full CLI integration
    • Vitest plugin for testing Angular stories
    • Compodoc integration for automatic component documentation
    • Multiple rendering strategies including TestBed support
    • Build and development server commands for Angular Vite projects
  • Tests

    • Comprehensive test coverage for Angular Vite integration

brandonroberts and others added 30 commits May 11, 2026 09:58
- Register ANGULAR_VITE in framework-to-renderer and framework-to-builder maps
- Add @storybook/angular-vite to frameworkPackages registry
- Update Angular generator to prompt for Vite vs Webpack builder (like Next.js)
- Update AngularJSON helper to write angular-vite builder entries
- Add ANGULAR_VITE to hasFrameworkTemplates list
- Add angular-cli/vite-default-ts sandbox template (inDevelopment)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
In the previewAnnotations transform, relative style imports were
written into the generated code via `resolve(projectRoot, extraImport)`,
which on Windows produces backslash-separated absolute paths. Those are
not valid module specifiers in `import '...';`.

Wrap the result with Vite's `normalizePath` so the specifier always uses
forward slashes. Also fix the `resolve` import — it was being pulled
from `node:url` (the deprecated URL resolver) instead of `node:path`.

Reported by JoostK in #34202.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Phase 2 of #34012 calls for "set default framework option for new
Angular projects using Vite". The Angular generator's `builderOverride`
was always prompting, which meant `storybook init --yes` (and any CI
run with no TTY) would either hang or pick whatever clack falls back
to instead of silently selecting the recommended Vite framework.

- Plumb `CommandOptions` through `builderOverride` so generators can
  see `--yes` and other CLI flags. The argument is optional, so the
  existing `() => ...` overrides (Next.js) keep working unchanged.
- Skip the Vite-vs-Webpack prompt in the Angular generator when
  `--yes` is set, `process.env.CI` is set, or stdin/stdout aren't a
  TTY, and return `SupportedBuilder.VITE` directly.

Interactive runs still get the prompt, so users on existing Angular
workspaces can opt back into the legacy webpack builder if they need
it.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…animations dep, sync EXISTING_RESOLUTIONS

- angular-cli/default-ts and angular-cli/prerelease now pass initOptions.builder=WEBPACK5
  so non-interactive storybook init does not auto-default to Vite.
- Generator extraPackages and sandbox extraDependencies now include @angular/animations
  (Angular 21 splits animations into a separate package; @angular/platform-browser
  imports legacy symbols from @angular/animations/browser).
- EXISTING_RESOLUTIONS includes rxjs to keep ecosystem-ci resolution sync test green.
…5 type

webpack 5.105 expanded Configuration['devtool'] to include the new
{ type, use: RawDevTool }[] variant. Forwarding the webpack type keeps
core-webpack and dependent presets in sync without manual maintenance.
…ypes

Importing webpack's Configuration type from the public entry pulls in
fast-uri/ajv typing that fails dts bundling. Inline the new devtool
shape directly instead.
Template stories were still importing from @storybook/angular, which is
not installed in vite sandboxes and broke rolldown bundling.
@analogjs/vite-plugin-angular requires every file with Angular
decorators to be part of the TS program. Stories alone are not enough
for components imported from sibling files; widen the include to all
TS files under src.
Vite 8's bundled rolldown@1.0.0 exports `Visitor`. @angular/build pulls
rolldown@1.0.0-rc.4 which does not, leading to runtime ESM link errors
when the preview server boots. Force a single rolldown across the
sandbox.
Vite 8 prefers oxc as transformer. Without explicit JSX runtime config,
React-only template stories (e.g. ResolvedReact.jsx) fail vite import
analysis with 'invalid JS syntax'. Apply the config in both build and
dev; keep keepNames build-only.
…azy-init bug

Rolldown's chunk splitting plus lazy-init wrappers produced a chunk
where _browser-chunk-* extends a class imported from _xhr-chunk-* but
the dependent init thunk was never invoked, leaving the imported class
undefined at evaluation. Coalescing @angular/platform-browser and
@angular/common into a single chunk via manualChunks keeps the class
hierarchy contiguous.
…onfig

@analogjs/vite-plugin-angular only compiles files that are part of the
tsconfig program. The sandbox tsconfig previously included only
`*.stories.ts` under `template-stories`, which left sibling component
sources (e.g. `pre.component.ts`, `form.component.ts`) outside the
program. Without compilation, the Angular `@Input()` / `@Output()`
decorators are stripped, `ɵcmp` metadata is never generated, and
`PropertyExtractor` falls back to an empty inputs/outputs list. That
causes `computesTemplateFromComponent` to emit a binding-free template
and template stories like `core-argmapping`, `module-mocking`,
`addon-actions`, and `addon-toolbars` to render empty content in the
angular-vite sandbox.

Widening the include glob to `../template-stories/**/*.ts` ensures
template components are compiled alongside the stories. The webpack
sandbox is unaffected because `@ngtools/webpack` resolves Angular
sources via the dependency graph rather than tsconfig include.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous broader `template-stories/**/*.ts` glob pulled in
`global.d.ts` which declares `globalThis.__STORYBOOK_ADDONS_CHANNEL__`
as `{ emit: any; on: any }`, breaking webpack typecheck for stories
that already used `channel.once`. It also picked up `utils.mock.ts`
which uses a `.ts` extension import unsupported by the sandbox
tsconfig. Restore the original `*.stories.ts` include and add only
`template-stories/components/**/*.ts`, which is where renderer template
components are symlinked. Keeps analogjs/vite-plugin-angular happy
without disturbing the webpack build.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…survives

The @analogjs/vite-plugin-angular `transform` hook replaces the incoming
`code` parameter with its own TS-compiled output (pulled from an internal
Angular file emitter) instead of forwarding the parameter through. With the
plugin sitting in Vite's "normal" group it ran after `storybook:mock-loader`
(which declares `transform.order: 'pre'`), so any automocked module body
produced by the mock plugin was silently discarded and stories rendered
the original implementation.

Force the main analogjs plugin into the "pre" group from `viteFinal` so it
transforms `.ts` sources before the mock plugin's automock pass, which then
operates on the compiled JS output. Verified locally on the angular-cli
vite sandbox: ModuleMocking / ModuleAutoMocking / NodeModuleMocking
stories now render mocked, automocked and node-module-mocked values
instead of the originals.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Without this, the @storybook/angular-vite sandbox skipped tsconfig
widening and compodoc/preserveSymlinks setup, so template-stories
components were never included in the Angular TS program.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cleanArgsDecorator filters args to those whose argType declares a
control or action. Compodoc-backed extraction populates argTypes by
class name, but rolldown rewrites class identifiers during the build,
so the lookup fails, argTypes stay empty, and every story arg the user
set was being discarded before reaching the Angular wrapper's
@input bindings. Skip the filter when argTypes is empty so the renderer
still receives the args produced by user decorators in build mode.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
valentinpalkovic and others added 16 commits June 1, 2026 15:17
…tation and improve config object retrieval logic
…c into a string framework

When main.ts uses `framework: '@storybook/angular-vite'` (a bare string),
`setFieldValue(['framework','options','compodoc'], false)` replaced the
string value with `{ options: { compodoc: false } }` and dropped the name.
Detect a string framework and rebuild it as `{ name, options }` so the name
survives. Object forms (including a getAbsolutePath-wrapped name) keep using
the nested set.
… builder

For the Vite framework, Compodoc is owned by the framework Vite plugin
(main.ts framework.options), so `init` should not write `compodoc`/
`compodocArgs` into the angular.json builder options. Add first-class
`frameworkOptions` support to the generator pipeline (promotes the main.ts
framework field to `{ name, options }`) and route Angular's compodoc there
for Vite. The Webpack framework keeps compodoc in angular.json as before.
…config file

The Vitest config templates resolve the Storybook config dir against the
generated config file's own directory (path.join(dirname, CONFIG_DIR)), but
postinstall filled CONFIG_DIR with the raw --config-dir value (relative to
the cwd the command ran in). In a monorepo where the config file is created
inside a subproject (e.g. apps/x/vitest.config.ts) and --config-dir
apps/x/.storybook is passed from the repo root, the path was doubled to
apps/x/apps/x/.storybook. Compute CONFIG_DIR relative to each config file's
directory via getTemplateConfigDir, across all three branches (workspace,
existing-config, fresh-create).

Also repairs a test invalidated by the function-notation merge refactor:
arrow-function configs are now merged + injected rather than deferred.
… until after install

The migration added addon-vitest/addon-a11y via `add(..., { skipInstall: true,
skipPostinstall: false })`, which invoked each addon's postinstall immediately.
But automigrate batches dependency installation to the end of the run, so the
addons were not yet on disk and `postinstallAddon` could not resolve their
postinstall hooks — silently no-opping. The addons ended up in main.ts and
package.json but were never configured.

Mirror CLI init's install-then-configure ordering: fixes now push core addon
names onto a new `addonsToConfigure` collector on RunOptions and add them with
`skipPostinstall: true`; the runner configures them after `installDependencies`,
when the packages can be resolved.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…lder prompt

Add an "in preview" hint to the @storybook/angular-vite option in the
Vite-vs-Webpack builder selection during `storybook init`, so users see the
preview status at the point of choosing the framework.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-to-angular-vite migration

When carrying a builder `compodoc: false` into `framework.options`, a framework
written as `getAbsolutePath('@storybook/angular')` was collapsed to
`framework: { options: { compodoc: false } }`, dropping the name.

`getFieldValue(['framework'])` evals the node, which throws on the
`getAbsolutePath(...)` call (not defined at eval time); the catch fell through to
a nested `setFieldValue(['framework','options','compodoc'], false)` that replaced
the whole value. Operate on the AST node instead via getFieldNode/setFieldNode:
string- and call-expression-form frameworks are wrapped as `{ name: <node>,
options: { compodoc: false } }`, preserving the original node (so getAbsolutePath
survives); object-form frameworks keep using the nested set.

Extract the logic into `setFrameworkCompodocFalse` and replace the brittle
mocked-field-setter tests (which missed this bug) with real-ConfigFile tests
covering string, getAbsolutePath, and object framework shapes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The deferred addon configuration was only wired into `doAutomigrate` (the
`storybook automigrate` path). `storybook upgrade` runs migrations through a
separate orchestrator (multi-project.ts -> runAutomigrationsForProjects), which
built its own RunOptions without `addonsToConfigure` and had no post-install
configuration step. As a result, the angular-to-angular-vite migration added
addon-vitest/addon-a11y to main.ts and package.json but their postinstall hooks
never ran (and `postinstallAddon` swallows resolution failures silently, so there
were no logs).

- Extract the shared `configureDeferredAddons` helper (loop + per-addon warn).
- multi-project: collect `addonsToConfigure` per project and surface it on
  AutomigrationResult.
- upgrade: after `installDependencies`, configure each project's deferred addons,
  mirroring CLI init's install-then-configure ordering.
- doAutomigrate: reuse the shared helper.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…amework detection

During `storybook upgrade`, the main config is evaluated early (to detect each
project's framework/version), which caches the pre-migration module via
importModule's URL-keyed cache. The angular-to-angular-vite migration then
rewrites main.ts to @storybook/angular-vite, but the deferred addon-vitest
postinstall called getStorybookInfo without skipping the cache, so it read the
stale @storybook/angular config. The prerequisite check then aborted with
"Non-Vite builder is not supported" / "Test feature cannot yet be used with
angular", even though main.ts already pointed at the Vite framework.

Thread `skipCache` through getStorybookInfo -> loadMainConfig and have the
addon-vitest postinstall pass it, so it always reflects the current on-disk
config after migrations.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve conflicts:
- code/core/src/common/versions.ts: take next's 10.5.0-alpha.5 versions, keep @storybook/angular-vite entry
- code/lib/cli-storybook/src/sandbox-templates.ts: take next's @angular/forms@21.2.16 pin and apply the same pin to the angular-cli/vite-default-ts template

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dRenderer flag

Re-architected port of #31311 (Angular: Component creation with TestBed API):

- TestBedRenderer mounts the existing story wrapper application through the
  documented TestBed singleton lifecycle (initTestEnvironment once or reused,
  resetTestingModule + configureTestingModule per render) instead of
  bootstrapApplication. A custom TestComponentRenderer places the fixture's
  host element inside the story target node, keeping :host styles, host
  listeners and selector semantics intact. fixture.autoDetectChanges() keeps
  change detection alive for play functions and storyProps$ updates.
- The mounting strategy is a single overridable seam (mountApplication) on
  AbstractRenderer, so input/output bindings, props updates and the rerender
  fast path are shared with the default renderer. Canvas-only: docs pages
  render multiple stories with different application configs, which cannot
  share the single TestBed environment injector.
- PropertyExtractor now hoists providers of ModuleWithProviders imports via
  importProvidersFrom and imports the plain NgModule instead - standalone
  wrapper components cannot import ModuleWithProviders, which previously
  broke such stories in both strategies.
- Add previewTestBedRenderer feature flag (core) and enable it in the
  angular-cli/vite-default-ts sandbox.
- Port template stories from #31311 (sanitizer, providers, rendering
  overrides, router) with fixes: inject() instead of type-only DI imports
  (erased by Vite type stripping, breaking JIT DI), provideRouter instead of
  a custom routing config API, RouterLink/RouterOutlet standalone imports.
- Rename template/stories_angular-cli-default-ts to
  stories_angular-cli-vite-default-ts so the signal stories actually get
  linked into the vite sandbox (sandbox story dirs are keyed by template id),
  and drop the never-linked stories_angular-cli-prerelease copy.
- Add @angular/router as optional peer dependency.
- Guard clearRootHTMLElement against missing root elements (vitest browser DOM).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…g; cover renderer selection with tests

provideRouter() relies on the application bootstrap to start the initial
navigation. TestBed.createComponent never bootstraps, so the mount util now
calls router.initialNavigation() when a Router is provided (documented
TestBed pattern), keeping @angular/router optional via dynamic import.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…virtual modules

analogjs's JIT support emits virtual modules for inline component styles
whose module id embeds the whole base64-encoded stylesheet. The bundler
derives chunk file names from the module id, which can exceed the OS's
255-byte file name limit and fail 'storybook build' with
'File name too long (os error 63)'. Trim the derived name to 64 chars in an
outputOptions plugin hook (survives rollupOptions/rolldownOptions config
normalization across vite versions) and let the content hash keep it unique.

Also add @angular/build to devDependencies - it is already a declared peer
and @analogjs/vite-plugin-angular resolves @angular/build/private from this
package's context in the linked/workspace setup.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Under @storybook/addon-vitest browser mode stories mount on synthetic
nodes while the TestBed singleton lifecycle is owned by the test runner -
its per-test resetTestingModule restores component definitions that were
scope-patched through our testing module, breaking module-declared
directives and pipes (NG0302) on subsequent renders. Use the TestBed
strategy only for the #storybook-root canvas and keep bootstrapApplication
for synthetic targets.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@valentinpalkovic valentinpalkovic added feature request angular builder-vite ci:normal Run our default set of CI jobs (choose this for most PRs). qa:needed Pull Requests that will need manual QA prior to release. labels Jun 9, 2026
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR adds Angular Vite as a supported framework, wires Angular-specific Vitest postinstall behavior, introduces the Angular Vite package and builders, adds Angular client renderer and docs extraction code, and includes framework templates, fixtures, and tests across setup, rendering, and documentation flows.

Changes

Angular Vite support

Layer / File(s) Summary
Vitest wiring and framework recognition
code/addons/vitest/src/*, code/core/src/babel/*, code/core/src/cli/*, code/core/src/common/utils/*, code/core/src/types/modules/*
Adds Angular Vite framework detection, supports callback-returning defineConfig object extraction, and injects storybookAngularVitest into matching Vitest plugin arrays during postinstall with new unit and integration coverage.
Framework package and builders
code/frameworks/angular-vite/package.json, code/frameworks/angular-vite/build*, code/frameworks/angular-vite/src/builders/*, code/frameworks/angular-vite/src/node/*, code/frameworks/angular-vite/src/types.ts
Introduces the @storybook/angular-vite package, public entrypoints, builder implementations and schemas, builder option/env bridging, compodoc execution helpers, and node/vitest exports.
Preset integration and workspace fixtures
code/frameworks/angular-vite/src/preset.ts, code/frameworks/angular-vite/src/client/globals.ts, code/frameworks/angular-vite/src/test-setup.ts, code/frameworks/angular-vite/src/server/*
Adds the Angular Vite preset, preview asset handling, Vite plugin composition, style and redirect transforms, and Angular/Nx workspace fixtures used by the framework tests.
Client API, preview typing, and decorators
code/frameworks/angular-vite/src/client/{index,config,preview,public-types,portable-stories,decorators,decorateStory,argsToTemplate,types}.ts, .../*.test*
Adds Angular client entrypoints, renderer-specific Storybook types, typed preview/meta/story factories, decorators, template-binding helpers, and related compile-time and runtime tests.
Renderer runtime and Angular mounting
code/frameworks/angular-vite/src/client/render.ts, .../renderer/*, .../renderer/utils/*, .../*.test.ts
Implements renderer selection for canvas/docs/TestBed modes, Angular application bootstrapping and rerender decisions, template generation from components, wrapper components, metadata extraction, and bootstrap serialization utilities with tests.
Compodoc extraction and docs support
code/frameworks/angular-vite/src/client/compodoc*.ts, code/frameworks/angular-vite/src/client/docs/*
Adds compodoc JSON types, argTypes and description extraction, docs source decorators/config, and fixture snapshots for component-property and model-signal documentation cases.
Templates, examples, and supporting tests
code/frameworks/angular-vite/template/..., code/frameworks/angular-vite/README.md, code/core/template/stories/order-of-hooks.stories.ts, code/e2e-sandbox/addon-actions.spec.ts
Adds Angular Vite CLI templates, reusable template components, example stories covering Angular patterns such as providers, module metadata, selectors, decorators, styles, docs, forms, sanitization, and updates a few supporting tests.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • storybookjs/storybook#34106: Both PRs update Vitest config AST handling in code/addons/vitest/src/updateVitestFile.ts and related config-shape support.
  • storybookjs/storybook#34403: Both PRs extend code/core/src/cli/AddonVitestService.constants.ts to add another supported framework to the Vitest addon flow.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

🟠 Major comments (24)
code/frameworks/angular-vite/src/client/renderer/AbstractRenderer.ts-194-202 (1)

194-202: ⚠️ Potential issue | 🟠 Major

Guard generated element names against invalid leading characters.

generateTargetSelectorFromStoryId returns id unchanged in the fast-path when it only matches [A-Za-z0-9-]; for story IDs like 1-foo, that value is then used in document.createElement(targetSelector) and can throw InvalidCharacterError (tag names can’t start with a digit), breaking rendering. Add a leading-character check (require starting [A-Za-z]) and route non-conforming ids through the existing sb-${cleaned}-component (or sb-story-component) fallback.

🤖 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/client/renderer/AbstractRenderer.ts` around
lines 194 - 202, The fast-path in generateTargetSelectorFromStoryId currently
returns id when it only contains [A-Za-z0-9-], but this still allows IDs
starting with digits (e.g., "1-foo") which will break document.createElement;
update generateTargetSelectorFromStoryId to require the id to start with a
letter (match /^[A-Za-z][A-Za-z0-9-]*$/) before returning it, and otherwise fall
through to the existing cleaning logic (invalidHtmlTag, cleaned) and return the
prefixed `sb-${cleaned}-component` or `sb-story-component` fallback.
code/frameworks/angular-vite/src/server/__mocks-ng-workspace__/with-options-styles/src/styles.css-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Remove the empty CSS block to avoid lint failure.

This fixture currently violates Stylelint block-no-empty.

Proposed fix
-.class {
-}
+/* Intentionally empty stylesheet fixture. */
🤖 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/server/__mocks-ng-workspace__/with-options-styles/src/styles.css`
around lines 1 - 2, Remove the empty CSS rule causing the Stylelint
block-no-empty error by deleting the ".class { }" block in
with-options-styles/src/styles.css (the selector ".class"); alternatively, if
the rule is intended to be used, replace the empty block with a valid
declaration (e.g., a real property) so the block is no longer empty.

Source: Linters/SAST tools

code/frameworks/angular-vite/template/stories/basics/component-with-complex-selectors/multiple-class-selector.component.stories.ts-1-1 (1)

1-1: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add explicit .ts file extension to relative import.

The import should use './multiple-selector.component.ts' instead of './multiple-selector.component'. As per coding guidelines, prefer explicit file extensions for relative code imports in this repository.

♻️ Proposed fix
-import { MultipleClassSelectorComponent } from './multiple-selector.component';
+import { MultipleClassSelectorComponent } from './multiple-selector.component.ts';
🤖 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/template/stories/basics/component-with-complex-selectors/multiple-class-selector.component.stories.ts`
at line 1, Change the relative import for MultipleClassSelectorComponent to
include the explicit .ts extension: replace the import that references
'./multiple-selector.component' with one that references
'./multiple-selector.component.ts' so the symbol MultipleClassSelectorComponent
is imported from the file with the explicit TypeScript extension.

Source: Coding guidelines

code/frameworks/angular-vite/src/client/docs/config.ts-4-4 (1)

4-4: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add explicit .ts file extension to relative import.

The import should use './sourceDecorator.ts' instead of './sourceDecorator'. As per coding guidelines, prefer explicit file extensions for relative code imports in this repository.

♻️ Proposed fix
-import { sourceDecorator } from './sourceDecorator';
+import { sourceDecorator } from './sourceDecorator.ts';
🤖 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/client/docs/config.ts` at line 4, Update the
relative import for the source decorator in config.ts to include the explicit
TypeScript file extension: replace the import of 'sourceDecorator' with
'sourceDecorator.ts' (the unique symbol to change is the import statement
referencing sourceDecorator in config.ts) so the module resolution uses the .ts
file extension per repository guidelines.

Source: Coding guidelines

code/frameworks/angular-vite/src/client/renderer/utils/NgComponentAnalyzer.ts-36-47 (1)

36-47: ⚠️ Potential issue | 🟠 Major

Parse aliased @Component inputs/outputs strings (prop: alias) correctly.

In code/frameworks/angular-vite/src/client/renderer/utils/NgComponentAnalyzer.ts, string inputs/outputs are currently treated verbatim as both propName and templateName, so prop: alias isn’t split and breaks binding-name detection for aliased entries.

Suggested fix
+const parseBindingName = (binding: string) => {
+  const [propRaw, aliasRaw] = binding.split(':');
+  const propName = propRaw.trim();
+  const templateName = (aliasRaw ?? propRaw).trim();
+  return { propName, templateName };
+};
+
 // Adds the I/O present in `@Component` metadata
 if (componentMetadata && componentMetadata.inputs) {
   initialValue.inputs.push(
     ...componentMetadata.inputs.map((i) => ({
-      propName: typeof i === 'string' ? i : i.name,
-      templateName: typeof i === 'string' ? i : i.alias,
+      ...(typeof i === 'string'
+        ? parseBindingName(i)
+        : { propName: i.name, templateName: i.alias ?? i.name }),
     }))
   );
 }
 if (componentMetadata && componentMetadata.outputs) {
   initialValue.outputs.push(
-    ...componentMetadata.outputs.map((i) => ({ propName: i, templateName: i }))
+    ...componentMetadata.outputs.map((i) =>
+      typeof i === 'string' ? parseBindingName(i) : { propName: i, templateName: i }
+    )
   );
 }
🤖 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/client/renderer/utils/NgComponentAnalyzer.ts`
around lines 36 - 47, The analyzer currently treats string entries in
componentMetadata.inputs/outputs as a single name, so aliased forms like "prop:
alias" aren't split; update the mapping in NgComponentAnalyzer where
initialValue.inputs and initialValue.outputs are populated to detect string
entries containing ":" and split on the first ":" (trim both sides) so propName
is the left side and templateName is the right side, and fall back to the single
name when no ":" is present; apply this same split logic for both the inputs
mapping (currently using typeof i === 'string' ? i : i.name / i.alias) and the
outputs mapping (currently using i verbatim) so alias bindings are parsed
correctly.
code/frameworks/angular-vite/template/stories/argTypes/doc-button/doc-button.component.ts-155-155 (1)

155-155: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use Storybook logger instead of console.log.

Lines 155, 204, 213, and 222 use console.log directly. As per coding guidelines, use storybook/internal/client-logger instead of raw console.* in normal code paths.

♻️ Proposed refactor

Add import at the top of the file:

 import type { ElementRef } from '`@angular/core`';
 import {
   Component,
   EventEmitter,
   HostBinding,
   HostListener,
   Input,
   Output,
   ViewChild,
 } from '`@angular/core`';
+import { logger } from 'storybook/internal/client-logger';

Then replace console.log calls:

   `@HostListener`('click', ['$event'])
   onClickListener(event: Event) {
-    console.log('button', event.target);
+    logger.log('button', event.target);
     this.handleClick(event);
   }
   /** A public method using an interface. */
   public publicMethod(things: ISomeInterface) {
-    console.log(things);
+    logger.log(things);
   }
   protected protectedMethod(id?: number) {
-    console.log(id);
+    logger.log(id);
   }
   private privateMethod(password: string) {
-    console.log(password);
+    logger.log(password);
   }

As per coding guidelines: Use Storybook loggers instead of raw console.* in normal code paths: server-side use storybook/internal/node-logger, client-side use storybook/internal/client-logger.

Also applies to: 204-204, 213-213, 222-222

🤖 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/template/stories/argTypes/doc-button/doc-button.component.ts`
at line 155, Replace raw console.log calls in doc-button.component.ts with
Storybook's client logger: add an import from 'storybook/internal/client-logger'
(e.g., import { logger } from 'storybook/internal/client-logger') at the top,
then change each console.log usage (the occurrences inside the component event
handlers — referenced by the string 'button' log and the other three console.log
calls at the same file) to use logger.info (or logger.debug as appropriate) so
logging uses Storybook's client-side logger; update the calls where event.target
or other values are passed to use logger.info('button', event.target) etc.,
keeping the same message and variables.

Source: Coding guidelines

code/frameworks/angular-vite/template/cli/page.component.ts-4-5 (1)

4-5: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add explicit file extensions to the relative imports.

Both relative imports should include explicit file extensions (.ts) to comply with the repository's coding guidelines for TypeScript source files.

♻️ Proposed fix
-import { HeaderComponent } from './header.component';
-import type { User } from './user';
+import { HeaderComponent } from './header.component.ts';
+import type { User } from './user.ts';

As per coding guidelines: "For TypeScript source in the repo, prefer explicit file extensions for relative code imports and exports such as ./foo.ts or ./bar.tsx when the target is another TS/JS module in this repository".

🤖 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/template/cli/page.component.ts` around lines 4 -
5, Update the two relative imports in page.component.ts so they include explicit
.ts extensions: change the import for HeaderComponent ('HeaderComponent') and
the type import for User ('User') to reference './header.component.ts' and
'./user.ts' respectively, ensuring both relative module paths use the .ts file
extension per the repo guideline.

Source: Coding guidelines

code/frameworks/angular-vite/template/stories/basics/component-with-complex-selectors/class-selector.component.stories.ts-1-1 (1)

1-1: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add explicit file extension to the relative import.

The import should include an explicit file extension (.ts or .js) to comply with the repository's coding guidelines for TypeScript source files.

♻️ Proposed fix
-import { ClassSelectorComponent } from './class-selector.component';
+import { ClassSelectorComponent } from './class-selector.component.ts';

As per coding guidelines: "For TypeScript source in the repo, prefer explicit file extensions for relative code imports and exports such as ./foo.ts or ./bar.tsx when the target is another TS/JS module in this repository".

🤖 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/template/stories/basics/component-with-complex-selectors/class-selector.component.stories.ts`
at line 1, Update the relative import for ClassSelectorComponent to include an
explicit TypeScript file extension: change the import statement that references
ClassSelectorComponent to import from './class-selector.component.ts' so it
follows the repo guideline requiring explicit .ts/.js extensions for TypeScript
source imports.

Source: Coding guidelines

code/frameworks/angular-vite/template/cli/header.component.ts-4-5 (1)

4-5: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add explicit file extensions to the relative imports.

Both relative imports should include explicit file extensions (.ts) to comply with the repository's coding guidelines for TypeScript source files.

♻️ Proposed fix
-import { ButtonComponent } from './button.component';
-import type { User } from './user';
+import { ButtonComponent } from './button.component.ts';
+import type { User } from './user.ts';

As per coding guidelines: "For TypeScript source in the repo, prefer explicit file extensions for relative code imports and exports such as ./foo.ts or ./bar.tsx when the target is another TS/JS module in this repository".

🤖 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/template/cli/header.component.ts` around lines 4
- 5, Update the two relative imports in header.component.ts to include explicit
.ts file extensions: change the import of ButtonComponent and the type import of
User so they reference './button.component.ts' and './user.ts' respectively;
ensure you only modify those import specifiers and leave the imported symbols
(ButtonComponent, User) and rest of the file unchanged to comply with the repo's
TypeScript import-extension guideline.

Source: Coding guidelines

code/frameworks/angular-vite/src/client/renderer/utils/NgModulesAnalyzer.ts-26-37 (1)

26-37: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard recursive module traversal against cycles.

Line 26 recursively walks imports with no cycle detection. If two modules reference each other, this can recurse indefinitely and overflow the stack.

Suggested fix
 export const isComponentAlreadyDeclared = (
   componentToFind: any,
   moduleDeclarations: any[],
-  moduleImports: any[]
+  moduleImports: any[],
+  visited = new Set<any>()
 ): boolean => {
@@
-  return moduleImports.flat().some((importItem) => {
+  return moduleImports.flat().some((importItem) => {
+    const target = importItem && importItem.ngModule ? importItem.ngModule : importItem;
+    if (visited.has(target)) {
+      return false;
+    }
+    visited.add(target);
+
     const extractedNgModuleMetadata = extractNgModuleMetadata(importItem);
     if (!extractedNgModuleMetadata) {
       // Not an NgModule
       return false;
     }
     return isComponentAlreadyDeclared(
       componentToFind,
       extractedNgModuleMetadata.declarations,
-      extractedNgModuleMetadata.imports
+      extractedNgModuleMetadata.imports,
+      visited
     );
   });
 };
🤖 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/client/renderer/utils/NgModulesAnalyzer.ts`
around lines 26 - 37, The recursive traversal can loop on cyclic imports; modify
the traversal to use a persistent visited Set keyed by the module identity
(e.g., the importItem symbol or module metadata canonical name) and pass it
through recursive calls so cycles are skipped. Concretely, update
isComponentAlreadyDeclared (or its caller) to accept an optional visited: Set,
at the start check if visited.has(importItem) and return false, add
visited.add(importItem) before recursing into
extractedNgModuleMetadata.imports/moduleImports, and ensure the same visited set
is passed to all nested calls that inspect imports; use extractNgModuleMetadata,
moduleImports, and importItem as the locations to add these checks/updates.
code/frameworks/angular-vite/src/client/renderer/RendererFactory.ts-12-21 (1)

12-21: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard targetDOMNode before dereferencing id

Line 16 reads targetDOMNode.id before the null guard at Line 20, so the documented null case will throw before the safeguard runs.

Suggested fix
   public async getRendererInstance(
-    targetDOMNode: HTMLElement,
+    targetDOMNode: HTMLElement | null,
     useTestBedRenderer = false
   ): Promise<AbstractRenderer | null> {
-    const targetId = targetDOMNode.id;
     // do nothing if the target node is null
     // fix a problem when the docs asks 2 times the same component at the same time
     // the 1st targetDOMNode of the 1st requested rendering becomes null 🤷‍♂️
     if (targetDOMNode === null) {
       return null;
     }
+    const targetId = targetDOMNode.id;
🤖 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/client/renderer/RendererFactory.ts` around
lines 12 - 21, In getRendererInstance, you currently read targetDOMNode.id
before checking for null which can throw; move the null/undefined guard to run
before any dereference (i.e., check if targetDOMNode is falsy and return null
early) and only assign const targetId = targetDOMNode.id after that guard so
that targetDOMNode is never dereferenced when null; update the function body
around the targetDOMNode variable to reflect this order change.
code/frameworks/angular-vite/src/client/renderer/DocsRenderer.ts-41-43 (1)

41-43: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Await application reset in beforeFullRender to avoid reset/render races.

beforeFullRender currently resolves before resetApplications finishes, so the next render can start against partially reset Angular state.

🔧 Suggested fix
   async beforeFullRender(domNode?: HTMLElement): Promise<void> {
-    DocsRenderer.resetApplications(domNode);
+    await DocsRenderer.resetApplications(domNode);
   }
🤖 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/client/renderer/DocsRenderer.ts` around
lines 41 - 43, beforeFullRender is async but calls
DocsRenderer.resetApplications(domNode) without awaiting, causing races; update
beforeFullRender to await the reset by changing the call to await
DocsRenderer.resetApplications(domNode) (and ensure resetApplications returns a
Promise/async if it isn’t already) so the method only resolves after
resetApplications completes.
code/frameworks/angular-vite/src/builders/build-storybook/index.spec.ts-33-33 (1)

33-33: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Do not keep this entire suite skipped.

describe.skip disables all validation for this builder path, so regressions can merge undetected.

🤖 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/builders/build-storybook/index.spec.ts` at
line 33, The test suite is entirely skipped via describe.skip which suppresses
all tests for the Build Storybook Builder; remove the skip so the suite runs
(replace describe.skip('Build Storybook Builder', ...) with describe('Build
Storybook Builder', ...)) and ensure any failing tests are fixed or individually
marked (use it.skip or test-specific skips) rather than skipping the whole
describe block; locate the describe.skip in index.spec.ts to make this change.
code/frameworks/angular-vite/src/builders/start-storybook/index.spec.ts-32-32 (1)

32-32: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Do not keep this entire suite skipped.

With describe.skip, the new start-storybook builder path is currently unverified.

🤖 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/builders/start-storybook/index.spec.ts` at
line 32, The test suite for the Start Storybook Builder is fully skipped via
describe.skip which prevents verification of the new builder; change
describe.skip('Start Storybook Builder', ...) to describe('Start Storybook
Builder', ...) (or remove the .skip) in index.spec.ts, re-enable any individual
it.skip tests only if they are known flaky, and run the spec (index.spec.ts) to
address any failures introduced by the builder so the suite actually verifies
the start-storybook path.
code/frameworks/angular-vite/src/builders/start-storybook/index.ts-219-223 (1)

219-223: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard tsConfig fallback when browserTarget is absent.

This path can dereference browserOptions.tsConfig while browserOptions is undefined.

Suggested patch
-  return {
-    tsConfig:
-      options.tsConfig ??
-      find.up('tsconfig.json', { cwd: options.configDir }) ??
-      browserOptions.tsConfig,
-  };
+  const tsConfig =
+    options.tsConfig ?? find.up('tsconfig.json', { cwd: options.configDir }) ?? browserOptions?.tsConfig;
+
+  if (!tsConfig) {
+    throw new Error('Unable to resolve tsConfig. Provide "tsConfig" or "browserTarget".');
+  }
+
+  return { tsConfig };
🤖 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/builders/start-storybook/index.ts` around
lines 219 - 223, The tsConfig fallback currently can dereference
browserOptions.tsConfig when browserOptions is undefined; update the expression
that builds tsConfig (the object containing tsConfig: options.tsConfig ??
find.up('tsconfig.json', { cwd: options.configDir }) ?? browserOptions.tsConfig)
to safely handle a missing browserOptions—e.g., use browserOptions?.tsConfig or
a conditional check (browserOptions && browserOptions.tsConfig) so the final
fallback won’t throw; locate this in the start-storybook builder where tsConfig
is composed and replace the raw access with the guarded form.
code/frameworks/angular-vite/src/builders/build-storybook/index.ts-175-179 (1)

175-179: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard tsConfig resolution before dereferencing browserOptions.

browserOptions can be undefined when no browserTarget is provided, so browserOptions.tsConfig can throw at runtime.

Suggested patch
-  return {
-    tsConfig:
-      options.tsConfig ??
-      find.up('tsconfig.json', { cwd: options.configDir, last: getProjectRoot() }) ??
-      browserOptions.tsConfig,
-  };
+  const tsConfig =
+    options.tsConfig ??
+    find.up('tsconfig.json', { cwd: options.configDir, last: getProjectRoot() }) ??
+    browserOptions?.tsConfig;
+
+  if (!tsConfig) {
+    throw new Error('Unable to resolve tsConfig. Provide "tsConfig" or "browserTarget".');
+  }
+
+  return { tsConfig };
🤖 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/builders/build-storybook/index.ts` around
lines 175 - 179, tsConfig resolution dereferences browserOptions which can be
undefined; guard it before use by checking browserOptions exists (or use
optional chaining) so you don't access browserOptions.tsConfig when
browserOptions is undefined. Update the expression that builds tsConfig (the
ternary chain using options.tsConfig, find.up(..., getProjectRoot()), and
browserOptions.tsConfig) to either use browserOptions?.tsConfig as the final
fallback or compute browserOptions earlier and only reference its tsConfig when
not null; ensure you still prefer options.tsConfig, then the find.up result,
then browserOptions's tsConfig if present.
code/frameworks/angular-vite/src/client/docs/angular-properties.test.ts-1-3 (1)

1-3: 🛠️ Refactor suggestion | 🟠 Major

Use memfs instead of direct node:fs reads in angular-properties.test.ts

  • This *.test.ts file reads fixture files using readFileSync/readdirSync from node:fs; switch to memfs (virtual FS) per the unit-test filesystem guideline.
  • globalThis.FEATURES.angularFilterNonInputControls is reset only at the end of the it(...) body; move the reset into finally/afterEach so it always runs if any snapshot/assertion fails.
🤖 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/client/docs/angular-properties.test.ts`
around lines 1 - 3, Tests in angular-properties.test.ts directly use node:fs
(readFileSync, readdirSync) and reset
globalThis.FEATURES.angularFilterNonInputControls only inside an it(...) body;
replace direct filesystem reads with memfs virtual FS APIs (mount the fixtures
into memfs before tests and use its read/dir calls) and ensure any mutation to
globalThis.FEATURES.angularFilterNonInputControls is always reverted by moving
the reset into an afterEach or a finally block so it runs even if the test
fails; locate uses of readFileSync/readdirSync and the it(...) that toggles
globalThis.FEATURES.angularFilterNonInputControls and update them accordingly.

Source: Coding guidelines

code/frameworks/angular-vite/src/builders/start-storybook/schema.json-99-103 (1)

99-103: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Loglevel pattern missing "error" option.

The description on Line 101 states the valid values include error: "silly, verbose, info (default), warn, error, silent", but the regex pattern on Line 102 is missing error:

"pattern": "(silly|verbose|info|warn|silent)"

This prevents users from setting loglevel to "error".

🐛 Proposed fix
     "loglevel": {
       "type": "string",
       "description": "Controls level of logging during build. Can be one of: [silly, verbose, info (default), warn, error, silent].",
-      "pattern": "(silly|verbose|info|warn|silent)"
+      "pattern": "(silly|verbose|info|warn|error|silent)"
     },
🤖 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/builders/start-storybook/schema.json` around
lines 99 - 103, The JSON schema for the "loglevel" property currently has a
pattern "(silly|verbose|info|warn|silent)" that omits "error"; update the
"pattern" value for the loglevel property to include "error" (e.g., add "|error"
into the alternation) so that valid values described in the "description"
(silly, verbose, info, warn, error, silent) are accepted; ensure the change is
applied to the "loglevel" property in schema.json.
code/addons/vitest/src/angular-vitest-postinstall.ts-74-83 (1)

74-83: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle aliased storybookAngularVitest imports before injecting the call.

With import { storybookAngularVitest as sbAngular } ..., Line 74-79 marks the import as present, but Line 166 injects storybookAngularVitest({}), which is unbound in that file.

Suggested fix
-function ensureImport(ast: BabelFile['ast']): void {
+function ensureImport(ast: BabelFile['ast']): string {
   let hasImport = false;
+  let localName = ANGULAR_VITEST_PLUGIN_CALL;

   traverse(ast, {
     ImportDeclaration(path) {
       if (path.node.source.value !== ANGULAR_VITEST_IMPORT_SOURCE) {
         return;
       }
-      const alreadyImports = path.node.specifiers.some(
+      const alreadyImports = path.node.specifiers.find(
         (specifier) =>
           specifier.type === 'ImportSpecifier' &&
           specifier.imported.type === 'Identifier' &&
           specifier.imported.name === ANGULAR_VITEST_PLUGIN_CALL
       );
       if (alreadyImports) {
+        if (alreadyImports.local?.type === 'Identifier') {
+          localName = alreadyImports.local.name;
+        }
         hasImport = true;
         path.stop();
       }
     },
   });

   if (hasImport) {
-    return;
+    return localName;
   }
@@
   const lastImportIndex = ast.program.body.findLastIndex((n) => n.type === 'ImportDeclaration');
   ast.program.body.splice(lastImportIndex + 1, 0, importDecl);
+  return localName;
 }

Also applies to: 106-107, 165-167

🤖 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/addons/vitest/src/angular-vitest-postinstall.ts` around lines 74 - 83,
The import-detection logic only checks imported.name ===
ANGULAR_VITEST_PLUGIN_CALL and treats presence as a match but ignores aliased
local names, so the injected call uses an unbound identifier; update the
detection to capture the local identifier when the named import is aliased
(i.e., when specifier.type === 'ImportSpecifier' && specifier.imported.name ===
ANGULAR_VITEST_PLUGIN_CALL) and store specifier.local.name (falling back to
ANGULAR_VITEST_PLUGIN_CALL if local is same or undefined); then change the
injection site that currently emits storybookAngularVitest({}) to use the
captured local name variable instead. Ensure the same change is applied to the
other detection/injection occurrences referenced by the comment (the blocks
around the existing alreadyImports checks and the call insertion near the
previous plugin-injection code).
code/frameworks/angular-vite/build-schema.json-32-36 (1)

32-36: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Anchor loglevel regex to validate the whole value.

Line 35 currently allows substring matches (e.g. warnx), so invalid values can pass schema validation.

Suggested fix
-      "pattern": "(trace|debug|info|warn|error|silent)"
+      "pattern": "^(trace|debug|info|warn|error|silent)$"
🤖 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/build-schema.json` around lines 32 - 36, The
"loglevel" JSON schema's pattern currently allows substring matches; update the
"loglevel" property's "pattern" (the "loglevel" schema entry) to anchor the
regex so it validates the entire string (e.g. use
^(trace|debug|info|warn|error|silent)$) ensuring values like "warnx" are
rejected.
code/addons/vitest/src/angular-vitest-postinstall.ts-54-57 (1)

54-57: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Restrict collected aliases to storybookTest import specifiers only.

Line 55 currently records any local specifier from @storybook/addon-vitest/vitest-plugin. That can mis-detect unrelated identifiers as valid plugin calls and inject into the wrong array.

Suggested fix
       path.node.specifiers.forEach((specifier) => {
-        if ('local' in specifier && specifier.local?.name) {
+        if (
+          specifier.type === 'ImportSpecifier' &&
+          specifier.imported.type === 'Identifier' &&
+          specifier.imported.name === STORYBOOK_TEST_PLUGIN_CALL &&
+          specifier.local?.name
+        ) {
           names.add(specifier.local.name);
         }
       });
🤖 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/addons/vitest/src/angular-vitest-postinstall.ts` around lines 54 - 57,
The current loop over path.node.specifiers adds every local specifier name (via
the local property) which can capture unrelated identifiers; change it to only
collect aliases for the specific exported symbol used by the plugin (the import
named "storybookTest" from "`@storybook/addon-vitest/vitest-plugin`"). In the
iteration over path.node.specifiers, check the specifier's imported name
(specifier.imported?.name or specifier.type === "ImportDefaultSpecifier" as
appropriate) and only call names.add(specifier.local.name) when the imported
name equals "storybookTest" (or when handling the default import case if your
plugin uses a default export). This ensures only the plugin's import alias is
collected.
code/frameworks/angular-vite/src/client/renderer/utils/BootstrapQueue.ts-46-52 (1)

46-52: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add error handling around bootstrap execution.

The processQueue loop lacks try-catch around the bootstrappingFn() call on line 49. If a bootstrap function throws synchronously (before returning a Promise), the error would propagate and stop queue processing, preventing subsequent queued bootstraps from executing.

🛡️ Proposed fix to add defensive error handling
   while (queue.length > 0) {
     const bootstrappingFn = queue.shift();
     if (bootstrappingFn) {
-      await bootstrappingFn();
-      await resetCompiledComponents();
+      try {
+        await bootstrappingFn();
+        await resetCompiledComponents();
+      } catch (e) {
+        // Error already handled by reject in queueBootstrapping; continue processing
+      }
     }
   }
🤖 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/client/renderer/utils/BootstrapQueue.ts`
around lines 46 - 52, The loop that executes queued bootstraps should guard each
call so one failing bootstrap doesn't stop the rest: inside the
while(queue.length>0) block wrap the await bootstrappingFn() call in try/catch
and ensure resetCompiledComponents() still runs (use finally or call reset after
handling the error); in the catch log the error with context (e.g., include
bootstrappingFn.name or an index) so failures are visible but processing
continues for remaining entries.
code/frameworks/angular-vite/template/stories/basics/component-with-complex-selectors/multiple-selector.component.ts-24-25 (1)

24-25: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Line 24 resolves the wrong factory class.

MultipleSelectorComponent currently resolves MultipleClassSelectorComponent, so selectors is populated from the wrong component metadata.

Suggested fix
-    const factory = this.resolver.resolveComponentFactory(MultipleClassSelectorComponent);
+    const factory = this.resolver.resolveComponentFactory(MultipleSelectorComponent);
🤖 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/template/stories/basics/component-with-complex-selectors/multiple-selector.component.ts`
around lines 24 - 25, The code is resolving the wrong factory: change the call
to this.resolver.resolveComponentFactory so it resolves
MultipleSelectorComponent (not MultipleClassSelectorComponent) so that
this.selectors is populated from the correct component metadata; update the
resolveComponentFactory(...) argument near where MultipleSelectorComponent,
MultipleClassSelectorComponent and this.selectors are referenced to use
MultipleSelectorComponent.
code/frameworks/angular-vite/src/builders/utils/run-compodoc.ts-13-15 (1)

13-15: ⚠️ Potential issue | 🟠 Major

Fix tsconfig relativization for Compodoc + support --tsconfig detection

  • toRelativePath() relativizes absolute tsconfig paths against . but Compodoc is executed with cwd: workspaceRoot; if process CWD differs from workspaceRoot, the -p path can be wrong (lines 13-15, 25, 39).
  • hasTsConfigArg() only checks for -p; Compodoc also supports --tsconfig, so passing --tsconfig can cause an extra -p to be injected (lines 7, 28).
Suggested fix
-const toRelativePath = (pathToTsConfig: string) => {
-  return isAbsolute(pathToTsConfig) ? relative('.', pathToTsConfig) : pathToTsConfig;
-};
+const toRelativePath = (pathToTsConfig: string, workspaceRoot: string) => {
+  return isAbsolute(pathToTsConfig)
+    ? relative(workspaceRoot || '.', pathToTsConfig)
+    : pathToTsConfig;
+};
@@
-  const tsConfigPath = toRelativePath(tsconfig);
+  const tsConfigPath = toRelativePath(tsconfig, workspaceRoot);
🤖 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/builders/utils/run-compodoc.ts` around lines
13 - 15, toRelativePath currently relativizes absolute tsconfig paths against
process.cwd(), which is wrong because Compodoc is executed with cwd:
workspaceRoot; update toRelativePath(pathToTsConfig: string, workspaceRoot:
string) (or use workspaceRoot from scope) so that if pathToTsConfig is absolute
you call relative(workspaceRoot, pathToTsConfig) and otherwise return as-is;
additionally update hasTsConfigArg() to detect both '-p' and '--tsconfig' flags
(and long/short forms with or without '=') so you don't inject an extra '-p'
when the caller already passed --tsconfig; adjust any call sites that build
Compodoc args (the logic that injects '-p' / uses toRelativePath, referenced in
the same module) to pass workspaceRoot into toRelativePath and to use the
updated flag-checking function.

selector: 'app-template',
imports: [CommonModule],
template: `
<div (click)="event($event)">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="code/frameworks/angular-vite/template/stories/basics/component-with-template/template.component.ts"

echo "== File exists? =="
ls -la "$FILE" || true

echo
echo "== Relevant lines (template.component.ts) =="
nl -ba "$FILE" | sed -n '1,160p'

echo
echo "== Search for `event(` usage in the component file (case-sensitive) =="
rg -n "event\s*\(" "$FILE" || true

echo
echo "== Search for `event` member declarations/usages in the component directory =="
DIR="$(dirname "$FILE")"
rg -n "\bevent\b" "$DIR" || true

Repository: storybookjs/storybook

Length of output: 339


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="code/frameworks/angular-vite/template/stories/basics/component-with-template/template.component.ts"
DIR="$(dirname "$FILE")"

echo "== Relevant file excerpt (template.component.ts) =="
# use cat -n if available; fallback to sed
if command -v cat >/dev/null 2>&1; then
  if cat -n "$FILE" >/dev/null 2>&1; then
    cat -n "$FILE" | sed -n '1,200p'
  else
    sed -n '1,200p' "$FILE"
  fi
else
  sed -n '1,200p' "$FILE"
fi

echo
echo "== Search for click bindings in this file =="
rg -n "\\(click\\)\\s*=\\s*\"" "$FILE" || true

echo
echo "== Search for `event(` usage in this file =="
rg -n "event\s*\(" "$FILE" || true

echo
echo "== Search for `event` member declarations/usages in the component directory =="
rg -n "\bevent\b" "$DIR" || true

echo
echo "== Search for `(click)=` usages in the directory (to see if template is shared) =="
rg -n "\\(click\\)\\s*=" "$DIR" || true

Repository: storybookjs/storybook

Length of output: 2026


Remove or implement the undefined (click) handler (event($event))

Template’s template binds the div click to event($event), but the Template class has no event handler, causing a runtime error when clicked.

Suggested fix
-    <div (click)="event($event)">
+    <div>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div (click)="event($event)">
<div>
🤖 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/template/stories/basics/component-with-template/template.component.ts`
at line 8, The template binds (click)="event($event)" but the Template class
lacks an event handler; either remove the (click) binding from the template or
implement a public method event(event: Event | MouseEvent) in the Template
component class (e.g., export class Template { event(evt: MouseEvent) { /*
handle or noop */ } }) so the click no longer throws a runtime error; update
Template to handle the event or remove the binding accordingly.

[displayText]="chip.text"
(removeClicked)="removeChipClick.emit(chip.id)"
></storybook-chip>
<div *ngIf="chips.length > 1" class="remove-all" (click)="removeAllChipsClick.emit()">

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Guard against undefined chips property.

The template accesses chips.length but chips is declared as optional (Line 42). If chips is undefined, this will throw a runtime error.

🛡️ Proposed fix
-    <div *ngIf="chips.length > 1" class="remove-all" (click)="removeAllChipsClick.emit()">
+    <div *ngIf="chips && chips.length > 1" class="remove-all" (click)="removeAllChipsClick.emit()">
       Remove All
     </div>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div *ngIf="chips.length > 1" class="remove-all" (click)="removeAllChipsClick.emit()">
<div *ngIf="chips && chips.length > 1" class="remove-all" (click)="removeAllChipsClick.emit()">
🤖 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/template/stories/basics/ng-module/angular-src/chips-group.component.ts`
at line 13, The template checks chips.length but chips is optional; update the
guard so the template won't throw when chips is undefined—either change the
*ngIf to use a safe check like chips?.length > 1 (or (chips && chips.length >
1)) or ensure the component initializes chips to an empty array in the class;
adjust the expression that triggers removeAllChipsClick.emit accordingly so it
only runs when chips is defined (refer to the chips property and the
removeAllChipsClick.emit usage in chips-group.component.ts).

@@ -0,0 +1,26 @@
import type { OnInit } from '@angular/core';
import { Component, Input } from '@angular/core';
import type { DummyService } from './dummy.service';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

target="code/frameworks/angular-vite/template/stories/core/moduleMetadata/angular-src/service.component.ts"
if [ ! -f "$target" ]; then
  echo "Missing file: $target" >&2
  exit 1
fi

echo "---- $target (first 120 lines) ----"
sed -n '1,140p' "$target" | nl -ba | sed -n '1,140p'

echo "---- Related dummy.service files ----"
# Find dummy.service.* near the import path
dir="$(dirname "$target")"
ls -la "$dir" | head -n 50
echo
fd -t f "dummy.service.*" "$dir" -a || true
fd -t f "dummy.service.*" "code/frameworks/angular-vite/template/stories/core/moduleMetadata/angular-src" -a || true

# Print the dummy service file(s) if found
for f in "$dir"/dummy.service.* "$dir"/dummy.service.ts "$dir"/dummy.service.js; do
  if [ -f "$f" ]; then
    echo "---- $f ----"
    sed -n '1,200p' "$f" | nl -ba | sed -n '1,200p'
  fi
done

Repository: storybookjs/storybook

Length of output: 254


🏁 Script executed:

#!/bin/bash
set -euo pipefail

target="code/frameworks/angular-vite/template/stories/core/moduleMetadata/angular-src/service.component.ts"
if [ ! -f "$target" ]; then
  echo "Missing file: $target" >&2
  exit 1
fi

echo "---- $target (first 200 lines) ----"
sed -n '1,220p' "$target" | nl -ba

echo "---- Related dummy.service files ----"
dir="$(dirname "$target")"
echo "Directory: $dir"
ls -la "$dir" | head -n 80

echo
echo "Searching for dummy.service.* under that directory..."
fd -t f "dummy.service.*" "$dir" || true

echo
# Print dummy service files (limit)
for f in $(fd -t f "dummy.service.*" "$dir" -a | head -n 20); do
  echo "---- $f ----"
  sed -n '1,200p' "$f" | nl -ba
  echo
done

Repository: storybookjs/storybook

Length of output: 254


🏁 Script executed:

#!/bin/bash
set -euo pipefail

target="code/frameworks/angular-vite/template/stories/core/moduleMetadata/angular-src/service.component.ts"
sed -n '1,220p' "$target" | nl -ba

dir="$(dirname "$target")"
echo "DIR: $dir"
ls -la "$dir" | head -n 60
fd -t f "dummy.service.*" "$dir" || true
for f in $(fd -t f "dummy.service.*" "$dir" -a | head -n 10); do
  echo "---- $f ----"
  sed -n '1,200p' "$f" | nl -ba
done

Repository: storybookjs/storybook

Length of output: 108


🏁 Script executed:

#!/bin/bash
set -euo pipefail

target="code/frameworks/angular-vite/template/stories/core/moduleMetadata/angular-src/service.component.ts"
echo "---- $target ----"
sed -n '1,220p' "$target" | nl -ba

dir="$(dirname "$target")"
echo "---- Directory listing: $dir ----"
ls -la "$dir" | head -n 60

echo "---- dummy.service.* files ----"
mapfile -t files < <(fd -t f "dummy.service.*" "$dir" || true)
printf "%s\n" "${files[@]}"

for f in "${files[@]}"; do
  echo "---- $f ----"
  sed -n '1,220p' "$f" | nl -ba
done

Repository: storybookjs/storybook

Length of output: 235


🏁 Script executed:

#!/bin/bash
set -euo pipefail

target="code/frameworks/angular-vite/template/stories/core/moduleMetadata/angular-src/service.component.ts"

echo "---- TARGET EXISTS? ----"
ls -ლა "$target" || exit 1

echo "---- $target (first 220 lines) ----"
awk '{printf "%5d\t%s\n", NR, $0}' "$target" | sed -n '1,220p'

dir="$(dirname "$target")"
echo
echo "---- Directory listing: $dir ----"
ls -la "$dir" | head -n 100

echo
echo "---- dummy.service.* files under $dir ----"
mapfile -t files < <(fd -t f "dummy.service.*" "$dir" 2>/dev/null || true)
if [ ${`#files`[@]} -eq 0 ]; then
  echo "(none found)"
else
  printf "%s\n" "${files[@]}"
fi

echo
for f in "${files[@]}"; do
  echo "---- $f (first 220 lines) ----"
  awk '{printf "%5d\t%s\n", NR, $0}' "$f" | sed -n '1,220p'
  echo
done

echo "---- Search for DummyService usage ----"
rg -n "DummyService" "$dir" || true

Repository: storybookjs/storybook

Length of output: 172


🏁 Script executed:

#!/bin/bash
set -euo pipefail

target="code/frameworks/angular-vite/template/stories/core/moduleMetadata/angular-src/service.component.ts"

echo "---- TARGET EXISTS? ----"
ls -la "$target" || exit 1

echo "---- $target (first 220 lines) ----"
awk '{printf "%5d\t%s\n", NR, $0}' "$target" | sed -n '1,220p'

dir="$(dirname "$target")"
echo
echo "---- Directory listing: $dir ----"
ls -la "$dir" | head -n 100

echo
echo "---- dummy.service.* files under $dir ----"
mapfile -t files < <(fd -t f "dummy.service.*" "$dir" 2>/dev/null || true)
if [ ${`#files`[@]} -eq 0 ]; then
  echo "(none found)"
else
  printf "%s\n" "${files[@]}"
fi

echo
for f in "${files[@]}"; do
  echo "---- $f (first 220 lines) ----"
  awk '{printf "%5d\t%s\n", NR, $0}' "$f" | sed -n '1,220p'
  echo
done

echo "---- Search for DummyService usage ----"
rg -n "DummyService" "$dir" || true

Repository: storybookjs/storybook

Length of output: 1765


🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="code/frameworks/angular-vite/template/stories/core/moduleMetadata/angular-src"
echo "---- dummy.service.ts ----"
awk '{printf "%5d\t%s\n", NR, $0}' "$base/dummy.service.ts" | sed -n '1,220p'

echo
echo "---- Search DummyService in directory ----"
rg -n "DummyService" "$base" || true

Repository: storybookjs/storybook

Length of output: 918


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find tsconfig files near the angular-vite template
base="code/frameworks/angular-vite/template"
echo "Searching tsconfig under $base ..."
find "$base" -name "tsconfig*.json" -maxdepth 6 2>/dev/null | sed 's|^\./||' | head -n 50

echo
echo "Global search for allowImportingTsExtensions ..."
rg -n "allowImportingTsExtensions" code -S || true

echo
echo "Also check for moduleResolution=Bundler / NodeNext (often required for .ts extension imports) ..."
rg -n "moduleResolution" code/frameworks/angular-vite/template -S || true

Repository: storybookjs/storybook

Length of output: 343


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "---- code/tsconfig.json (relevant excerpt) ----"
# Print around allowImportingTsExtensions and module settings
awk 'NR>=1 && NR<=80 {print}' code/tsconfig.json
echo
rg -n "allowImportingTsExtensions" -n code/tsconfig.json

echo
echo "---- Find tsconfig(s) under angular-vite template ----"
find code/frameworks/angular-vite/template -name "tsconfig*.json" -maxdepth 8 2>/dev/null | head -n 50

echo
echo "---- Repo-wide search for Angular constructor injection with import type ----"
rg -n "import type\s*\{[^}]*\}\s*from\s*['\"][^'\"]+['\"];[\s\S]{0,200}constructor\([^)]*:\s*[A-Za-z0-9_]+\)" code/frameworks -S || true

echo
echo "---- Narrow search: import type + constructor(private ...: DummyService) ----"
rg -n "import type\s*\{[^}]*DummyService[^}]*\}\s*from\s*['\"][^'\"]+['\"][\s\S]{0,200}constructor\([^)]*DummyService" code -S || true

Repository: storybookjs/storybook

Length of output: 1199


🏁 Script executed:

#!/bin/bash
set -euo pipefail
base="code/frameworks"

echo "---- Search for 'import type' in angular framework templates ----"
rg -n "import type\s*\{" "$base" --glob "*/angular-*/**/*.{ts,tsx}" || true

echo
echo "---- Search for 'constructor(private ...: ' with nearby 'import type' (same file) ----"
# Print matches for 'import type' in files that also contain 'constructor(private'
files=$(rg -l "import type\s*\{" "$base" --glob "*.{ts,ts,tsx}" || true)
for f in $files; do
  if rg -q "constructor\s*\(\s*private\s+[^:]+:\s*[^)]+\)" "$f"; then
    echo "---- $f ----"
    rg -n "import type\s*\{|constructor\s*\(\s*private" "$f" | head -n 20
  fi
done

echo
echo "---- Search for 'import type' and 'emitDecoratorMetadata' interplay (optional) ----"
rg -n "emitDecoratorMetadata" code -S || true

Repository: storybookjs/storybook

Length of output: 3893


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "---- tsconfig under code/frameworks/angular-vite ----"
find code/frameworks/angular-vite -name "tsconfig*.json" -maxdepth 6 2>/dev/null | head -n 50

echo
echo "---- Search emitDecoratorMetadata in angular-vite ----"
rg -n "emitDecoratorMetadata" code/frameworks/angular-vite -S || true

echo
echo "---- code/frameworks/angular-vite/tsconfig.json (excerpt) ----"
if [ -f code/frameworks/angular-vite/tsconfig.json ]; then
  awk 'NR>=1 && NR<=120 {print}' code/frameworks/angular-vite/tsconfig.json
fi

echo
echo "---- Search for tsconfig in the angular-vite template folder ----"
find code/frameworks/angular-vite/template -name "tsconfig*.json" -maxdepth 8 2>/dev/null || true

Repository: storybookjs/storybook

Length of output: 3642


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "from ['\"]\\./[^'\"]+\\.ts['\"]" "code/frameworks/angular-vite/template" --glob "*.{ts,tsx}" || true
rg -n "from ['\"]\\./[^'\"]+\\.ts['\"]" "code/frameworks/angular/template" --glob "*.{ts,tsx}" || true
rg -n "from ['\"]\\./[^'\"]+\\.ts['\"]" "code/frameworks/angular-vite" --glob "*.{ts,tsx}" | head -n 50 || true

Repository: storybookjs/storybook

Length of output: 5991


Fix Angular DI constructor injection: replace import type with a value import for DummyService

ServiceComponent constructor injects DummyService, but the import is type-only, so the runtime class needed for Angular’s decorated constructor parameter metadata won’t be available. Use a value import (and follow the repo’s explicit .ts extension convention).

Suggested fix
-import type { DummyService } from './dummy.service';
+import { DummyService } from './dummy.service.ts';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import type { DummyService } from './dummy.service';
import { DummyService } from './dummy.service.ts';
🤖 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/template/stories/core/moduleMetadata/angular-src/service.component.ts`
at line 3, The constructor of ServiceComponent injects DummyService but the file
currently uses a type-only import; change the import to a value import so the
DummyService class is present at runtime (replace "import type { DummyService }"
with a normal import and include the explicit ".ts" extension per repo
convention) so Angular's DI metadata for the ServiceComponent constructor can be
emitted correctly.

Source: Coding guidelines

@valentinpalkovic valentinpalkovic marked this pull request as draft June 9, 2026 20:52
valentinpalkovic and others added 3 commits June 9, 2026 23:06
…g before wrapper creation

The TestBed strategy was previously disabled for synthetic (non
#storybook-root) targets because module-declared directives and pipes broke
with NG0302 under addon-vitest browser mode. The actual root cause was an
ordering bug in our own mount, not a conflict with the test runner:

resetTestingModule() calls resetCompiledComponents(), which clears Angular's
global JIT module scoping queue. Our mount ran the reset AFTER render() had
already created and decorated the story's wrapper component and its
declarations NgModule - wiping the just-enqueued module before the wrapper
compiled, so its declared components never received their transitive scope
(directiveDefs/pipeDefs). The bug was masked outside the full vitest suite
because TestBed's one-time checkGlobalCompilationFinished() flushes the
queue on the very first reset of a page - which is exactly the first render
in a fresh preview iframe or a single-file vitest run, but long gone by the
time a later story renders in a shared page. The same latent bug affected
the preview when navigating to such a story after another one.

Move the reset into TestBedRenderer.beforeFullRender(), which runs before
the wrapper and module classes are created. The fresh module then enqueues
after the wipe and is flushed by the standard JIT path when the wrapper
component compiles (compilationDepth === 0) - the same order the
bootstrapApplication strategy relies on. The renderer selection no longer
special-cases synthetic targets.

Verified with cleared vite caches: 315/315 sandbox vitest browser tests,
unit tests, prod build, and the order-sensitive preview scenario (navigate
from another story to the pipe story in the same page).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Resolve conflict in code/core/src/common/versions.ts (next bumped to
10.5.0-alpha.6); bump @storybook/angular-vite to 10.5.0-alpha.6 to match.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@storybook-app-bot

Copy link
Copy Markdown

Package Benchmarks

Commit: c3091f2, ran on 12 June 2026 at 09:10:01 UTC

The following packages have significant changes to their size or dependencies:

storybook

Before After Difference
Dependency count 72 72 0
Self size 20.86 MB 20.74 MB 🎉 -114 KB 🎉
Dependency size 36.12 MB 36.12 MB 0 B
Bundle Size Analyzer Link Link

@storybook/angular-vite

Before After Difference
Dependency count 0 31 🚨 +31 🚨
Self size 0 B 25.15 MB 🚨 +25.15 MB 🚨
Dependency size 0 B 19.84 MB 🚨 +19.84 MB 🚨
Bundle Size Analyzer Link Link

@storybook/cli

Before After Difference
Dependency count 203 203 0
Self size 947 KB 960 KB 🚨 +13 KB 🚨
Dependency size 89.11 MB 88.91 MB 🎉 -205 KB 🎉
Bundle Size Analyzer Link Link

@storybook/codemod

Before After Difference
Dependency count 196 196 0
Self size 32 KB 32 KB 🚨 +36 B 🚨
Dependency size 87.60 MB 87.39 MB 🎉 -214 KB 🎉
Bundle Size Analyzer Link Link

create-storybook

Before After Difference
Dependency count 73 73 0
Self size 1.08 MB 1.09 MB 🚨 +9 KB 🚨
Dependency size 56.97 MB 56.86 MB 🎉 -114 KB 🎉
Bundle Size Analyzer node node

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

angular builder-vite ci:normal Run our default set of CI jobs (choose this for most PRs). feature request qa:needed Pull Requests that will need manual QA prior to release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants