Angular: Migrate from RxJS to async/await in command builders and run Compodoc utility as spinner #33156
Conversation
valentinpalkovic
force-pushed
the
valentin/show-compodoc-progress
branch
from
7aed235 to
298effd
Compare
|
View your CI Pipeline Execution ↗ for commit ed50d99
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughThree Angular Storybook builder modules were converted from RxJS Observable-based flows to async/await Promise-based implementations: builders' entry points and runInstance functions now return Promises, and runCompodoc now returns Promise. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI Entrypoint
participant BuilderOld as Builder (Observable)
participant RxJS as RxJS Pipeline
participant RunOld as runInstance / runCompodoc (old)
participant BuilderNew as Builder (async)
participant RunNew as runInstance / runCompodoc (new)
participant Result as BuilderOutput
Note over BuilderOld,RxJS: Old (Observable-based)
CLI->>BuilderOld: invoke commandBuilder()
BuilderOld->>RxJS: create Observable pipeline
RxJS->>RunOld: subscribe -> runInstance / runCompodoc
RunOld-->>RxJS: emit values / complete / error
RxJS-->>BuilderOld: map/mapTo results
BuilderOld-->>CLI: return Observable stream (emits BuilderOutput)
Note over BuilderNew,RunNew: New (async/await)
CLI->>BuilderNew: await commandBuilder()
BuilderNew->>RunNew: await runCompodoc / runInstance()
RunNew-->>BuilderNew: return value or throw
BuilderNew-->>CLI: return Promise resolved with BuilderOutput
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
✨ Finishing touches
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (4)**/*.{ts,tsx,js,jsx,mjs}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx,json,html,mjs}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
code/**/!(*.test).{ts,tsx,js,mjs}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-11-24T17:49:47.649ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (4)
Comment |
valentinpalkovic
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
code/frameworks/angular/src/builders/utils/run-compodoc.ts (1)
18-44: AsyncrunCompodocrefactor looks solid; consider aligning package-manager cwdThe async/await flow and
prompt.executeTaskWithSpinnerwrapper look correct and keep errors propagating via the returnedPromise<void>, while satisfying the logging guideline by usingstorybook/internal/node-loggerrather thanconsole. One small robustness improvement would be to align package-manager detection with the execution cwd, e.g.:const packageManager = JsPackageManagerFactory.getPackageManager( undefined, context.workspaceRoot ?? process.cwd() );so that lockfile detection and
runPackageCommand({ cwd: context.workspaceRoot })operate from the same root, which can matter in multi-workspace or non-standard invocation setups.code/frameworks/angular/src/builders/start-storybook/index.ts (1)
75-177: Guard againstNaNwhen normalizingoptions.portThe async builder flow looks good overall, and the compodoc +
--silenthandling is nicely integrated. One small robustness tweak:options.port = parseInt(\${options.port}`, 10);will produceNaNifoptions.portis everundefined/nullat this stage (e.g., no CLI arg and no env override), which could then be passed through tobuildDevStandalone`. You may want to default or guard here, for example:const rawPort = options.port ?? 6006; // or whatever default Storybook CLI uses options.port = parseInt(String(rawPort), 10);to ensure a valid numeric port is always used.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
code/frameworks/angular/src/builders/build-storybook/index.ts(3 hunks)code/frameworks/angular/src/builders/start-storybook/index.ts(3 hunks)code/frameworks/angular/src/builders/utils/run-compodoc.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use camelCase for variable and function names
Files:
code/frameworks/angular/src/builders/utils/run-compodoc.tscode/frameworks/angular/src/builders/start-storybook/index.tscode/frameworks/angular/src/builders/build-storybook/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Enable TypeScript strict mode
Export functions from modules for testing purposes
Files:
code/frameworks/angular/src/builders/utils/run-compodoc.tscode/frameworks/angular/src/builders/start-storybook/index.tscode/frameworks/angular/src/builders/build-storybook/index.ts
**/*.{ts,tsx,js,jsx,json,html,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,json,html,mjs}: Use ESLint and Prettier for code style enforcement
Run 'yarn prettier --write ' to format code after making changes
Run 'yarn lint:js:cmd ' to check for ESLint issues after making changes
Files:
code/frameworks/angular/src/builders/utils/run-compodoc.tscode/frameworks/angular/src/builders/start-storybook/index.tscode/frameworks/angular/src/builders/build-storybook/index.ts
code/**/!(*.test).{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/!(*.test).{ts,tsx,js,mjs}: Use 'logger' from 'storybook/internal/node-logger' for server-side (Node.js) logging, not console.log/console.warn/console.error
Use 'logger' from 'storybook/internal/client-logger' for client-side (browser) logging, not console.log/console.warn/console.error
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/frameworks/angular/src/builders/utils/run-compodoc.tscode/frameworks/angular/src/builders/start-storybook/index.tscode/frameworks/angular/src/builders/build-storybook/index.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T17:49:47.649Z
Learning: The Storybook repository is a TypeScript/React monorepo with main codebase in 'code/' directory and tooling in 'scripts/' directory
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
📚 Learning: 2025-11-24T17:49:47.649Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T17:49:47.649Z
Learning: Applies to code/**/!(*.test).{ts,tsx,js,mjs} : Use 'logger' from 'storybook/internal/node-logger' for server-side (Node.js) logging, not console.log/console.warn/console.error
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.tscode/frameworks/angular/src/builders/start-storybook/index.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/frameworks/angular/src/builders/start-storybook/index.ts
📚 Learning: 2025-11-24T17:49:47.649Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T17:49:47.649Z
Learning: Use 'cd code && yarn storybook:ui' to start the development server for testing UI changes on http://localhost:6006/
Applied to files:
code/frameworks/angular/src/builders/start-storybook/index.ts
📚 Learning: 2025-11-24T17:49:47.649Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T17:49:47.649Z
Learning: The Storybook repository is a TypeScript/React monorepo with main codebase in 'code/' directory and tooling in 'scripts/' directory
Applied to files:
code/frameworks/angular/src/builders/build-storybook/index.ts
🧬 Code graph analysis (2)
code/frameworks/angular/src/builders/utils/run-compodoc.ts (1)
code/core/src/common/js-package-manager/JsPackageManagerFactory.ts (1)
JsPackageManagerFactory(29-201)
code/frameworks/angular/src/builders/build-storybook/index.ts (4)
code/frameworks/angular/src/builders/utils/run-compodoc.ts (1)
runCompodoc(18-45)code/core/src/common/utils/cli.ts (1)
getEnvConfig(89-97)code/core/src/core-server/withTelemetry.ts (1)
withTelemetry(147-203)code/frameworks/angular/src/builders/utils/error-handler.ts (1)
errorSummary(23-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
code/frameworks/angular/src/builders/build-storybook/index.ts (2)
70-149: AsynccommandBuilderflow reads clearly and preserves behaviorThe migration to an
asynccommandBuilderthat awaitssetup, conditionally awaitsrunCompodoc, appliesgetEnvConfig, and then builds a singleStandaloneBuildOptionsobject before callingrunInstanceis straightforward and keeps the previous behavior while making the control flow much easier to follow. No issues spotted in the sequencing or option wiring.
173-193:runInstancetelemetry + error summarization looks correctWrapping
withTelemetry('build', …)in atry/catchand converting failures tonew Error(errorSummary(error))keeps detailed logging/telemetry inwithTelemetrywhile surfacing a concise, user-facing message up to the Angular builder. Ignoring the return value fromwithTelemetryis fine here since only success/failure matters forbuild-storybook.code/frameworks/angular/src/builders/start-storybook/index.ts (1)
200-219: ConfirmwithTelemetry('dev', …)always yields an object withportThe async
runInstancewrapper aroundwithTelemetry('dev', …)andbuildDevStandaloneis a clean replacement for the previous Observable flow, and the error summarization viaerrorSummaryis consistent with the build builder. The line:const { port } = await withTelemetry(/* ... */);assumes that for the
'dev'event type,withTelemetrywill always resolve to an object containingport(and neverundefined), otherwise destructuring would throw before your owntry/catchsees the error. If that invariant is guaranteed elsewhere, this is fine; otherwise consider a minimal guard or assertion to make the assumption explicit.
valentinpalkovic
force-pushed
the
valentin/show-compodoc-progress
branch
from
298effd to
43bac1c
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts (1)
17-23: Mock implementation correctly aligned with async/await pattern.The mock for
executeTaskWithSpinnerproperly executes the task function and returns a Promise (as an async function). The implementation correctly supports the test scenarios.Optional: Consider stricter typing for the mock.
The
fnparameter is typed asany. While acceptable for test mocks, you could make it more specific:executeTaskWithSpinner: async (fn: () => Promise<any>) => { await fn(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
code/frameworks/angular/src/builders/build-storybook/index.ts(3 hunks)code/frameworks/angular/src/builders/start-storybook/index.ts(3 hunks)code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts(6 hunks)code/frameworks/angular/src/builders/utils/run-compodoc.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern*.test.ts,*.test.tsx,*.spec.ts, or*.spec.tsx
Follow the spy mocking rules defined in.cursor/rules/spy-mocking.mdcfor consistent mocking patterns with Vitest
Files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use camelCase for variable and function names
Files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.tscode/frameworks/angular/src/builders/build-storybook/index.tscode/frameworks/angular/src/builders/utils/run-compodoc.tscode/frameworks/angular/src/builders/start-storybook/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Enable TypeScript strict mode
Export functions from modules for testing purposes
Files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.tscode/frameworks/angular/src/builders/build-storybook/index.tscode/frameworks/angular/src/builders/utils/run-compodoc.tscode/frameworks/angular/src/builders/start-storybook/index.ts
**/*.{ts,tsx,js,jsx,json,html,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,json,html,mjs}: Use ESLint and Prettier for code style enforcement
Run 'yarn prettier --write ' to format code after making changes
Run 'yarn lint:js:cmd ' to check for ESLint issues after making changes
Files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.tscode/frameworks/angular/src/builders/build-storybook/index.tscode/frameworks/angular/src/builders/utils/run-compodoc.tscode/frameworks/angular/src/builders/start-storybook/index.ts
code/**/!(*.test).{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
code/**/!(*.test).{ts,tsx,js,mjs}: Use 'logger' from 'storybook/internal/node-logger' for server-side (Node.js) logging, not console.log/console.warn/console.error
Use 'logger' from 'storybook/internal/client-logger' for client-side (browser) logging, not console.log/console.warn/console.error
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.tscode/frameworks/angular/src/builders/build-storybook/index.tscode/frameworks/angular/src/builders/utils/run-compodoc.tscode/frameworks/angular/src/builders/start-storybook/index.ts
🧠 Learnings (24)
📓 Common learnings
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors in Vitest tests
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies in Vitest tests
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mock implementations outside of `beforeEach` blocks in Vitest tests
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in `beforeEach` blocks in Vitest tests
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:47.649Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T17:49:47.649Z
Learning: Applies to **/*.test.{ts,tsx} : Use 'vi.mock()' to mock external dependencies like file system and loggers in unit tests
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses in Vitest tests
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases in Vitest tests
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together in Vitest tests
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed in Vitest tests
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid direct function mocking without `vi.mocked()` in Vitest tests
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mock()` with the `spy: true` option for all package and file mocks in Vitest tests
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:47.649Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T17:49:47.649Z
Learning: Applies to code/**/!(*.test).{ts,tsx,js,mjs} : Use 'logger' from 'storybook/internal/node-logger' for server-side (Node.js) logging, not console.log/console.warn/console.error
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.tscode/frameworks/angular/src/builders/utils/run-compodoc.tscode/frameworks/angular/src/builders/start-storybook/index.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use type-safe mocking with `vi.mocked()` in Vitest tests
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use `vi.mocked()` to type and access the mocked functions in Vitest tests
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:47.649Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T17:49:47.649Z
Learning: Applies to code/**/!(*.test).{ts,tsx,js,mjs} : Use 'logger' from 'storybook/internal/client-logger' for client-side (browser) logging, not console.log/console.warn/console.error
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking without the `spy: true` option in Vitest tests
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:59.268Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.268Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused in Vitest tests
Applied to files:
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts
📚 Learning: 2025-11-24T17:49:47.649Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T17:49:47.649Z
Learning: The Storybook repository is a TypeScript/React monorepo with main codebase in 'code/' directory and tooling in 'scripts/' directory
Applied to files:
code/frameworks/angular/src/builders/build-storybook/index.ts
📚 Learning: 2025-11-05T09:38:47.712Z
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Select/Select.tsx:200-204
Timestamp: 2025-11-05T09:38:47.712Z
Learning: Repo: storybookjs/storybook — Guidance: Until Storybook 11 is released, do not suggest using React.useId anywhere (e.g., in code/core/src/components/components/Select/Select.tsx) to maintain compatibility with React 17 runtimes. Prefer advising: accept a caller-provided props.id and, if needed, generate a client-only fallback id to minimize SSR hydration issues — but avoid useId. Resume prompting for useId after Storybook 11.
Applied to files:
code/frameworks/angular/src/builders/start-storybook/index.ts
📚 Learning: 2025-11-24T17:49:47.649Z
Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T17:49:47.649Z
Learning: Use 'cd code && yarn storybook:ui' to start the development server for testing UI changes on http://localhost:6006/
Applied to files:
code/frameworks/angular/src/builders/start-storybook/index.ts
🧬 Code graph analysis (4)
code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts (1)
code/frameworks/angular/src/builders/utils/run-compodoc.ts (1)
runCompodoc(18-45)
code/frameworks/angular/src/builders/build-storybook/index.ts (5)
code/frameworks/angular/src/builders/start-storybook/index.ts (1)
StorybookBuilderOptions(37-71)code/frameworks/angular/src/builders/utils/run-compodoc.ts (1)
runCompodoc(18-45)code/core/src/common/utils/cli.ts (1)
getEnvConfig(89-97)code/core/src/core-server/withTelemetry.ts (1)
withTelemetry(147-203)code/frameworks/angular/src/builders/utils/error-handler.ts (1)
errorSummary(23-33)
code/frameworks/angular/src/builders/utils/run-compodoc.ts (1)
code/core/src/common/js-package-manager/JsPackageManagerFactory.ts (1)
JsPackageManagerFactory(29-201)
code/frameworks/angular/src/builders/start-storybook/index.ts (4)
code/frameworks/angular/src/builders/utils/run-compodoc.ts (1)
runCompodoc(18-45)code/core/src/common/utils/cli.ts (1)
getEnvConfig(89-97)code/core/src/core-server/withTelemetry.ts (1)
withTelemetry(147-203)code/frameworks/angular/src/builders/utils/error-handler.ts (1)
errorSummary(23-33)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (6)
code/frameworks/angular/src/builders/utils/run-compodoc.ts (1)
18-44: LGTM! Clean migration from RxJS to async/await with spinner UX.The conversion from Observable-based flow to Promise-based async/await is well executed. The spinner wrapper (
prompt.executeTaskWithSpinner) enhances user experience by providing clear feedback during Compodoc generation.code/frameworks/angular/src/builders/utils/run-compodoc.spec.ts (1)
46-52: LGTM! Tests correctly updated to await Promise instead of subscribing to Observable.All test cases properly use
awaitwith the Promise-basedrunCompodocfunction, correctly reflecting the migration from RxJS Observables to async/await.Also applies to: 61-67, 76-82, 91-97, 106-112
code/frameworks/angular/src/builders/build-storybook/index.ts (2)
70-148: LGTM! Successful migration from RxJS pipeline to async/await flow.The
commandBuilderfunction is cleanly converted to an async function with a clear sequential flow:
- Setup (awaited)
- Compodoc execution (conditionally awaited)
- Environment configuration
- Build options preparation
- Build execution via
runInstance(awaited)The migration maintains all original functionality while improving code readability and maintainability.
173-192: LGTM! Error handling properly migrated to try/catch pattern.The
runInstancefunction correctly wraps the build execution in a try/catch block, preserving error handling semantics while using async/await. The error summary is applied before rethrowing, maintaining the original error reporting behavior.code/frameworks/angular/src/builders/start-storybook/index.ts (2)
75-176: LGTM! Clean migration to async/await with proper port handling.The
commandBuilderfunction successfully migrates to async/await with a clear sequential flow. The port extraction fromrunInstanceand inclusion in the returnedBuilderOutputis correctly implemented. The addition of--silentflag to compodocArgs whenoptions.quietis true is a nice touch for consistent logging behavior.
200-218: LGTM! Error handling and port extraction correctly implemented.The
runInstancefunction properly uses async/await with try/catch error handling. The port is correctly extracted from thewithTelemetryresult and returned to the caller. Error summarization before rethrowing maintains consistent error reporting.
Closes #
What I did
Running Storybook for Angular hasn't indicated during startup that Compodoc is built, leading to the impression that the CLI got stuck. This got fixed by using a spinner for Compodoc's progress.
Additionally, the Angular builders were refactored to be asynchronous rather than using rxjs.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
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.tsMake 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.🦋 Canary release
This pull request has been released as version
0.0.0-pr-33156-sha-ed50d99f. Try it out in a new sandbox by runningnpx [email protected] sandboxor in an existing project withnpx [email protected] upgrade.More information
0.0.0-pr-33156-sha-ed50d99fvalentin/show-compodoc-progressed50d99f1764057695)To request a new release of this pull request, mention the
@storybookjs/coreteam.core team members can create a new canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=33156Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.