CSF-Factories: Fix ConfigFile parser false warning on definePreview({...}).type<T>() export default#33885
Conversation
Co-authored-by: valentinpalkovic <5889929+valentinpalkovic@users.noreply.github.com>
definePreview({...}).type<T>() export default
|
View your CI Pipeline Execution ↗ for commit 2bdb54a
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit f48a1ae
☁️ Nx Cloud last updated this comment at |
|
@copilot Fix linting issues as described in .github/copilot-instructions.md |
definePreview({...}).type<T>() export defaultdefinePreview({...}).type<T>() export default
📝 WalkthroughWalkthroughEnhanced CSF factory declaration parsing by introducing iterative unwrapping logic to handle nested patterns like Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing Touches
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/core/src/csf-tools/ConfigFile.test.ts`:
- Around line 296-312: The test must assert that no warning is emitted during
parse for the .type<T>() chaining case: wrap the call to
loadConfig(source).parse() with a spy on the warning emitter (e.g.,
jest.spyOn(console, 'warn') or the test logger used by loadConfig), call parse
via loadConfig(source).parse(), then expect the spy not to have been called, and
finally restore the spy; reference the existing loadConfig(...) and parse()
calls so you add the spy before parse and the expect(spy).not.toHaveBeenCalled()
after.
| it('parses correctly with .type<T>() chaining on export default', () => { | ||
| const source = dedent` | ||
| import { definePreview } from '@storybook/react-vite'; | ||
|
|
||
| export default definePreview({ | ||
| parameters: { | ||
| foo: 'bar', | ||
| }, | ||
| }).type<{ | ||
| parameters: { | ||
| customParam?: string; | ||
| }; | ||
| }>(); | ||
| `; | ||
| const config = loadConfig(source).parse(); | ||
| expect(config.getFieldValue(['parameters', 'foo'])).toEqual('bar'); | ||
| }); |
There was a problem hiding this comment.
Assert warning suppression explicitly in this regression test.
This test only validates parsed data; it can still pass even if the parser logs the false warning again. Please also assert that no warning is emitted during parse for this .type<T>() pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/core/src/csf-tools/ConfigFile.test.ts` around lines 296 - 312, The test
must assert that no warning is emitted during parse for the .type<T>() chaining
case: wrap the call to loadConfig(source).parse() with a spy on the warning
emitter (e.g., jest.spyOn(console, 'warn') or the test logger used by
loadConfig), call parse via loadConfig(source).parse(), then expect the spy not
to have been called, and finally restore the spy; reference the existing
loadConfig(...) and parse() calls so you add the spy before parse and the
expect(spy).not.toHaveBeenCalled() after.
Co-authored-by: valentinpalkovic <5889929+valentinpalkovic@users.noreply.github.com>
…ser-warning
CSF-Factories: Fix ConfigFile parser false warning on `definePreview({...}).type<T>()` export default
(cherry picked from commit 011e081)
ConfigFileparser warns ondefinePreview({...}).type<{...}>()inexport defaultExportDefaultDeclarationhandler inConfigFile.tsto unwrap chained calls like.type<T>()definePreview({...}).type<{...}>()inConfigFile.test.tsConfigFile.tsOriginal prompt
ConfigFileparser warns ondefinePreview({...}).type<{...}>()inexport default#33798💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Summary by CodeRabbit
Bug Fixes
Tests