fix(storybook-angular): forward applyDecorators in testing#2236
Conversation
✅ Deploy Preview for analog-blog ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for analog-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR adds a new local Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Key observationsAPI source change: Implementation logic density: The Test coverage breadth: Test cases include basic template generation and output binding preservation, but template generation logic has multiple branches (void element detection, attribute formatting, binding filtering) that may benefit from additional edge-case coverage. Monorepo consistency: Changes follow the pattern of adding implementation + corresponding 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This fix broken storybook and was reverted #2086 (comment) |
…-component-wrapper-decorator
I investigated this further and confirmed the concern about the earlier The previous fix from add missing applyDecorators to render annotations analogjs/analog#2086 does fix the I reproduced that locally through the portable-story/Vitest path. When Angular's This PR no longer forwards I also added targeted regression coverage for:
Verified with |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/storybook-angular/src/lib/component-wrapper-decorator.spec.ts (1)
4-4: Add one lightweight public-API export assertion.Since this PR also changes the re-export in
packages/storybook-angular/src/index.ts, add a small test importing from../indexto lock package-entry compatibility.Suggested test addition
import { describe, expect, it, vi } from 'vitest'; import { componentWrapperDecorator } from './component-wrapper-decorator'; +import { componentWrapperDecorator as publicComponentWrapperDecorator } from '../index'; @@ describe('componentWrapperDecorator', () => { + it('is re-exported from the package entrypoint', () => { + expect(publicComponentWrapperDecorator).toBe(componentWrapperDecorator); + }); + it('prepares the base story template before wrapping portable stories', () => {As per coding guidelines, "
**/*.spec.{ts,tsx}: Keep tests lightweight and targeted to critical functionality testing."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/storybook-angular/src/lib/component-wrapper-decorator.spec.ts` at line 4, Add a lightweight public-API export test that imports componentWrapperDecorator from the package entry (import { componentWrapperDecorator } from '../index') and asserts it is defined; update the spec file component-wrapper-decorator.spec.ts to import from '../index' (not just './component-wrapper-decorator') and add a simple expect(componentWrapperDecorator).toBeDefined() to lock the public re-export via the package entry point.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/storybook-angular/src/lib/component-wrapper-decorator.spec.ts`:
- Line 4: Add a lightweight public-API export test that imports
componentWrapperDecorator from the package entry (import {
componentWrapperDecorator } from '../index') and asserts it is defined; update
the spec file component-wrapper-decorator.spec.ts to import from '../index' (not
just './component-wrapper-decorator') and add a simple
expect(componentWrapperDecorator).toBeDefined() to lock the public re-export via
the package entry point.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 97d8a8e8-1ded-490e-9b9a-a2bf6537fff1
📒 Files selected for processing (4)
packages/storybook-angular/src/index.tspackages/storybook-angular/src/lib/component-wrapper-decorator.spec.tspackages/storybook-angular/src/lib/component-wrapper-decorator.tspackages/storybook-angular/src/lib/testing.spec.ts
✅ Files skipped from review due to trivial changes (1)
- packages/storybook-angular/src/lib/testing.spec.ts
PR Checklist
Closes #2074
Affected scope
storybook-angularRecommended merge strategy for maintainer [optional]
Commit preservation note [optional]
What is the new behavior?
This brings the
componentWrapperDecoratorfix ontoanalogjs/alpha.The same
applyDecoratorsfix already landed previously in analogjs/analog#2086, but that PR merged intobeta. The currentalphabranch still lacks the change inpackages/storybook-angular/src/lib/testing.ts, so portable Vitest story rendering onalphastill falls back to the generic Storybook decorator path.This PR forwards
configAnnotations.applyDecoratorsthroughsetProjectAnnotations()and adds a focused regression test to lock that behavior in.Reproduction reference from the maintainer thread:
#2074 (comment)
Test plan
nx format:checkpnpm buildpnpm testCommands run:
pnpm nx test storybook-angular --runTestsByPath packages/storybook-angular/src/lib/testing.spec.tspnpm nx build storybook-angularDoes this PR introduce a breaking change?
Other information
The new regression test is the main value beyond reapplying the known fix: it makes sure
setProjectAnnotations()keeps passingapplyDecoratorsthrough on thealphaline.