CLI: Add Stencil Integration #32768
CLI: Add Stencil Integration #32768
Conversation
artursopelnik
requested review from
jonniebigodes and
kylegach
as code owners
📝 WalkthroughWalkthroughStencil framework support is integrated across the codebase by adding the ProjectType.STENCIL enum, including 'stencil' in supported renderers and frameworks type unions, configuring Vite as the default builder, creating a generator module, adding template configurations, and updating documentation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes The changes are homogeneous and follow consistent patterns across files: adding 'stencil' to existing maps, type unions, and configuration objects without introducing new control flow or complex logic. The additive nature and repetitive structure across multiple files simplifies review. Possibly related PRs
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
code/core/src/cli/detect.test.ts (1)
229-238: Consider updating test fixture to use current version of @stencil/core.The latest stable version of @stencil/core is 4.36.3, but the test fixture uses version 4.30.0. While this is a valid historical version and the test will work correctly, test fixtures should ideally reflect current package versions to avoid testing with outdated dependencies. Consider updating the fixture to use version 4.36.3 or another recent stable version.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
README.md(1 hunks)code/core/src/cli/detect.test.ts(1 hunks)code/core/src/cli/detect.ts(1 hunks)code/core/src/cli/helpers.ts(1 hunks)code/core/src/cli/project_types.ts(4 hunks)code/core/src/common/utils/framework-to-renderer.ts(1 hunks)code/core/src/types/modules/frameworks.ts(1 hunks)code/core/src/types/modules/renderers.ts(1 hunks)code/lib/cli-storybook/src/sandbox-templates.ts(2 hunks)code/lib/create-storybook/src/bin/modernInputs.ts(3 hunks)code/lib/create-storybook/src/generators/STENCIL/index.ts(1 hunks)code/lib/create-storybook/src/initiate.ts(2 hunks)docs/get-started/install.mdx(3 hunks)scripts/create-nx-sandbox-projects.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
code/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
code/**/*.{test,spec}.{ts,tsx}: Place all test files under the code/ directory
Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Files:
code/core/src/cli/detect.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)
**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access mocked functions
Implement mock behaviors in beforeEach blocks
Mock all required dependencies that the test subject uses
Mock implementations should be placed in beforeEach blocks
Each mock implementation should return a Promise for async functions
Mock implementations should match the expected return type of the original function
Use vi.mocked() to access and implement mock behaviors
Mock all required properties and methods that the test subject uses
Avoid direct function mocking without vi.mocked()
Avoid mock implementations outside of beforeEach blocks
Avoid mocking without the spy: true option
Avoid inline mock implementations within test cases
Avoid mocking only a subset of required dependencies
Mock at the highest level of abstraction needed
Keep mock implementations simple and focused
Use type-safe mocking with vi.mocked()
Document complex mock behaviors
Group related mocks together
Files:
code/core/src/cli/detect.test.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier formatting on changed files before committing
Run ESLint on changed files and fix all errors/warnings before committing (useyarn lint:js:cmd <file>)
Files:
code/core/src/cli/detect.test.tsscripts/create-nx-sandbox-projects.tscode/core/src/cli/helpers.tscode/lib/create-storybook/src/generators/STENCIL/index.tscode/core/src/types/modules/renderers.tscode/core/src/cli/detect.tscode/lib/create-storybook/src/initiate.tscode/lib/cli-storybook/src/sandbox-templates.tscode/core/src/cli/project_types.tscode/core/src/types/modules/frameworks.tscode/core/src/common/utils/framework-to-renderer.tscode/lib/create-storybook/src/bin/modernInputs.ts
**/*.@(test|spec).{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.@(test|spec).{ts,tsx,js,jsx}: Unit tests should import and execute the functions under test rather than only asserting on syntax patterns
Mock external dependencies in tests usingvi.mock()(e.g., filesystem, loggers)
Files:
code/core/src/cli/detect.test.ts
**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Export functions from modules when they need to be unit-tested
Files:
code/core/src/cli/detect.test.tsscripts/create-nx-sandbox-projects.tscode/core/src/cli/helpers.tscode/lib/create-storybook/src/generators/STENCIL/index.tscode/core/src/types/modules/renderers.tscode/core/src/cli/detect.tscode/lib/create-storybook/src/initiate.tscode/lib/cli-storybook/src/sandbox-templates.tscode/core/src/cli/project_types.tscode/core/src/types/modules/frameworks.tscode/core/src/common/utils/framework-to-renderer.tscode/lib/create-storybook/src/bin/modernInputs.ts
code/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In application code, use Storybook loggers instead of
console.*(client code:storybook/internal/client-logger; server code:storybook/internal/node-logger)
Files:
code/core/src/cli/detect.test.tscode/core/src/cli/helpers.tscode/lib/create-storybook/src/generators/STENCIL/index.tscode/core/src/types/modules/renderers.tscode/core/src/cli/detect.tscode/lib/create-storybook/src/initiate.tscode/lib/cli-storybook/src/sandbox-templates.tscode/core/src/cli/project_types.tscode/core/src/types/modules/frameworks.tscode/core/src/common/utils/framework-to-renderer.tscode/lib/create-storybook/src/bin/modernInputs.ts
{code/**,scripts/**}/**/*.{ts,tsx,js,jsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not use
console.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/core/src/cli/detect.test.tsscripts/create-nx-sandbox-projects.tscode/core/src/cli/helpers.tscode/lib/create-storybook/src/generators/STENCIL/index.tscode/core/src/types/modules/renderers.tscode/core/src/cli/detect.tscode/lib/create-storybook/src/initiate.tscode/lib/cli-storybook/src/sandbox-templates.tscode/core/src/cli/project_types.tscode/core/src/types/modules/frameworks.tscode/core/src/common/utils/framework-to-renderer.tscode/lib/create-storybook/src/bin/modernInputs.ts
scripts/**/*.{ts,js,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
In Node.js scripts, use
storybook/internal/node-loggerinstead ofconsole.*
Files:
scripts/create-nx-sandbox-projects.ts
**/README.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Update package or feature README files when making significant changes
Files:
README.md
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Update documentation under
docs/for significant changes, including migration guides for breaking changes
Files:
docs/get-started/install.mdx
🧬 Code graph analysis (1)
code/lib/create-storybook/src/generators/STENCIL/index.ts (2)
code/lib/create-storybook/src/generators/types.ts (1)
Generator(35-40)code/lib/create-storybook/src/generators/baseGenerator.ts (1)
baseGenerator(222-482)
🪛 markdownlint-cli2 (0.18.1)
README.md
113-113: Images should have alternate text (alt text)
(MD045, no-alt-text)
🔇 Additional comments (21)
scripts/create-nx-sandbox-projects.ts (1)
22-24: LGTM! Consistent handling of external Stencil plugin.The addition of
'@stencil/storybook-plugin'to the exclusion list follows the same pattern used for other external frameworks like Qwik and SolidJS, ensuring proper dependency handling for community-maintained plugins.code/core/src/cli/detect.ts (1)
138-138: LGTM! Correct builder selection for STENCIL.The addition of
ProjectType.STENCILto the Vite builder fallback is consistent with theframeworkToDefaultBuildermapping inhelpers.ts(line 156) and appropriately groups STENCIL with other Vite-based frameworks.code/core/src/common/utils/framework-to-renderer.ts (1)
34-34: LGTM! Renderer mapping correctly added.The
stenciltostencilmapping is properly placed alphabetically and follows the same pattern as other renderer entries.docs/get-started/install.mdx (3)
156-156: LGTM! Framework type table updated correctly.The Stencil entry in the CLI framework detection table is properly formatted and consistent with other framework entries.
293-296: LGTM! Troubleshooting section added for Stencil users.The IfRenderer block for Stencil follows the same pattern as other renderer-specific troubleshooting sections and provides appropriate links to the Stencil Storybook repository and community support.
319-319: LGTM! Renderer array updated to include Stencil.The addition of
'stencil'to the IfRenderer's renderer array ensures that the conditional content displays correctly for Stencil projects.code/lib/create-storybook/src/initiate.ts (2)
54-54: LGTM! Generator import correctly added.The
stencilGeneratorimport is properly placed alphabetically among other framework generator imports.
199-202: LGTM! STENCIL project type handler correctly implemented.The
ProjectType.STENCILcase follows the exact same pattern as other framework cases, properly calling the generator and logging an appropriate message.code/core/src/cli/helpers.ts (1)
156-156: LGTM! Default builder mapping correctly configured.The
stencil: CoreBuilder.Vitemapping is properly placed alphabetically and aligns with the builder detection logic indetect.ts(line 138), ensuring consistent Vite builder selection for Stencil projects.code/lib/create-storybook/src/generators/STENCIL/index.ts (1)
4-6: LGTM!The generator implementation correctly delegates to
baseGeneratorwith 'stencil' as both the renderer and framework. This follows the established pattern for framework generators.code/core/src/types/modules/frameworks.ts (1)
21-22: LGTM!The addition of 'stencil' to
SupportedFrameworksis consistent with the other framework entries.code/lib/create-storybook/src/bin/modernInputs.ts (3)
19-19: LGTM!Adding 'stencil' to the
supportedFrameworksarray is correct and maintains alphabetical ordering within the array.
49-49: LGTM!The package mapping to '@stencil/storybook-plugin' is consistent with the external framework configuration.
73-73: LGTM!The display name 'Stencil' is clear and consistent with the naming convention used for other frameworks.
code/core/src/types/modules/renderers.ts (1)
15-15: LGTM!The addition of 'stencil' to
SupportedRenderersis consistent with the other renderer entries.code/lib/cli-storybook/src/sandbox-templates.ts (2)
667-679: LGTM!The Stencil template configuration is well-structured with appropriate flags:
inDevelopment: trueprevents CI usage until the template is production-ready- Extensive
skipTaskslist accounts for missing standard stories- The expected framework/renderer/builder all pointing to '@stencil/storybook-plugin' matches the pattern used by other community frameworks like Qwik
890-890: LGTM!Commenting out 'stencil/default-ts' in the daily templates is appropriate given the
inDevelopment: trueflag.code/core/src/cli/project_types.ts (4)
35-39: LGTM!The external framework configuration for Stencil is correctly structured and consistent with similar frameworks like Qwik.
52-52: LGTM!Adding 'stencil' to the
SUPPORTED_RENDERERSarray is correct and necessary for renderer support.
79-79: LGTM!The
ProjectType.STENCILenum member addition is consistent with other framework types.
258-264: Move STENCIL template above the "DO NOT MOVE" comment or clarify constraint intent.Stencil projects do not require React as a dependency, so the original concern about detection order is unfounded��there's no risk of a Stencil project matching the REACT template first.
However, the placement issue remains: STENCIL is positioned below the "DO NOT MOVE ANY TEMPLATES BELOW THIS LINE" comment at line 242, which appears to violate the documented constraint. While WEBPACK_REACT and REACT templates are also below this line, moving STENCIL above it would:
- Clarify whether the constraint applies to all templates or only React-related ones
- Maintain logical grouping with other early-detection frameworks
- Avoid confusion about intentional constraint violations
Either move STENCIL above line 242, or add a clarifying comment explaining why this template is exempt from the "DO NOT MOVE" constraint.
|
View your CI Pipeline Execution ↗ for commit 684fd76
☁️ Nx Cloud last updated this comment at |
docs/get-started/install.mdx
Outdated
| </IfRenderer> | ||
|
|
||
| <IfRenderer renderer={['angular', 'vue', 'web-components', 'ember', 'html', 'svelte', 'preact', 'qwik', 'solid' ]}> | ||
| <IfRenderer renderer={['angular', 'vue', 'web-components', 'ember', 'html', 'svelte', 'preact', 'qwik', 'solid', 'stencil' ]}> |
There was a problem hiding this comment.
@artursopelnik, the same applies here as already mentioned regarding the documentation rendering
docs/get-started/install.mdx
Outdated
| <IfRenderer renderer="stencil"> | ||
| * [Storybook's Stencil README](https://github.com/stenciljs/storybook) for more information on how to set up Storybook in your Stencil project. | ||
| * [Storybook's help documentation](https://storybook.js.org/community#support) to contact the community and ask for help. | ||
| </IfRenderer> |
There was a problem hiding this comment.
@artursopelnik, quick item here, if you don't mind. This will require a change in the Storybook documentation website to allow it to render when a user selects the Stencil framework, as we currently don't have support for it. This may lead to inconsistent documentation rendering. Until that happens, we can rely on the renderer="web-components"to allow the documentation to render effectively, wouldn't you agree?
There was a problem hiding this comment.
@jonniebigodes yes, let's update the documentation later once the feature has been implemented.
3 participants
What I did
This PR continues the work started in the previous branch #31205 from @christian-bromann
It aims to integrate a Stencil installer into Storybook’s CLI, enabling users to easily set up Stencil projects with Storybook.
If we can get everything working well, I would be happy to add the Stencil installer officially to Storybook’s CLI.
Following up on comments from #31205 from @shilman , I’ve made some adjustments to fix the following issues:
CLI project detection for Stencil
CLI using Vite as the default for Stencil projects
I believe the last two points may already be fixed in newer versions, as users should be on an updated Storybook 9 version.
Summary by CodeRabbit
New Features
Documentation