-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: CSF factories #30197
base: next
Are you sure you want to change the base?
WIP: CSF factories #30197
Changes from 13 commits
9415e47
66c8234
9fc6fbe
cdda2b5
5d05b3c
628e657
79107a5
60c6f26
c2988dc
6b0abf0
b4e6e25
c370fec
1d2ee2f
d04b258
410f988
25e9575
c061e82
3466750
84f2ae6
3753cde
5f7c6af
3eead18
5efce1e
dd5d1be
b70fdcc
e584c17
1f3600e
03a8b9c
b1bd157
ea5b8cb
03f8b11
07dcd78
e0ef30e
309a435
86fe532
49353cb
ec0c55f
ef92a43
745f1b5
30511e9
f59916d
eb50508
ddbe8b9
4127d87
740c149
5349a7d
f4e3a62
0c7bace
23bcf14
76b1b6c
0ae3643
4d2b108
53864bd
6d08dd1
dd7fd84
126cd95
19f4b0d
861625a
80ccd47
f151fef
5fe2c12
f1a2c53
6c27056
659930e
7db0050
ebe6bf9
df000bb
5ac85ef
778405d
e68b8f9
1f79546
b0972bc
325a554
9df03e6
37811bb
503f883
d4e3695
6648198
211fb4b
310ebf2
f38a9f6
5c3752a
04d9f58
71fefda
69941ca
5b4cd43
80c80ac
2ec88ee
3a60679
cc571a4
ae4128b
5e0b320
e2f0eb2
78d2d93
99c86eb
69a5282
9bbaee4
62e4b08
25ad641
27e0406
bf9f008
5e0e2c2
9be57ad
fb806c2
7945ae6
2711770
2c565f7
c585eef
e69943b
8d1c77f
44e01cb
e71bade
1cd8644
3f6a53e
2c13a66
7967016
5c6599e
488fd4f
b068e8b
ab9329e
43e3c4e
cab9679
5b68c6a
a3d613d
700ec5a
c81b6a0
8ceedc1
02a4069
876fbc0
46de186
23c49db
2085d4b
b93d96a
fccc0df
bd5f88c
c5cb4b1
209cdeb
d5b592a
1f5a406
1f4c92f
1a356cb
03ec593
cecfd85
815c778
43ac425
70429c2
a838e73
dc79392
3847a11
b5b0420
aa305bb
ce00ea5
d0c668b
66131f1
1d637ec
580f1e2
025b64f
c862158
de6ae64
e3b1620
7e96e46
636346d
e383e7d
b1c03f6
0f00a18
6e89d67
9349e5b
29c6263
1b9a3ea
359ec9c
9760a3e
1f39998
04a004b
eab4926
8def9e0
4820815
7b4f819
e9c4ae8
2e92d97
8babc1d
08bb053
e21a07b
39deab4
de13df0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,11 +27,12 @@ export const testStory = ( | |
return async (context: TestContext & TaskContext & { story: ComposedStoryFn }) => { | ||
const composedStory = composeStory( | ||
story, | ||
meta, | ||
'isCSFFactory' in story ? (meta as any).annotations : meta, | ||
{ initialGlobals: (await getInitialGlobals?.()) ?? {} }, | ||
undefined, | ||
exportName | ||
); | ||
|
||
if (composedStory === undefined || skipTags?.some((tag) => composedStory.tags.includes(tag))) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Optional chaining on skipTags is unnecessary since it's a required parameter |
||
context.skip(); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,7 @@ export async function generateModernIframeScriptCode(options: Options, projectRo | |
[], | ||
options | ||
); | ||
const previewAnnotationURLs = [...previewAnnotations, previewOrConfigFile] | ||
const [previewFileUrl, ...previewAnnotationURLs] = [...previewAnnotations, previewOrConfigFile] | ||
.filter(Boolean) | ||
.map((path) => processPreviewAnnotation(path, projectRoot)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Destructuring could fail if previewAnnotations and previewOrConfigFile are both empty arrays after filtering. Add a check to handle this case. |
||
|
||
|
@@ -23,14 +23,23 @@ export async function generateModernIframeScriptCode(options: Options, projectRo | |
// modules are provided, the rest are null. We can just re-import everything again in that case. | ||
const getPreviewAnnotationsFunction = ` | ||
const getProjectAnnotations = async (hmrPreviewAnnotationModules = []) => { | ||
const preview = await import('${previewFileUrl}'); | ||
const csfFactoryPreview = Object.values(preview).find(module => { | ||
return 'isCSFFactoryPreview' in module | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Object.values() and find() could return undefined. Need type checking and error handling for when no CSF factory preview is found. |
||
|
||
if (csfFactoryPreview) { | ||
return csfFactoryPreview.annotations; | ||
} | ||
|
||
const configs = await Promise.all([${previewAnnotationURLs | ||
.map( | ||
(previewAnnotation, index) => | ||
// Prefer the updated module from an HMR update, otherwise import the original module | ||
`hmrPreviewAnnotationModules[${index}] ?? import('${previewAnnotation}')` | ||
) | ||
.join(',\n')}]) | ||
return composeConfigs(configs); | ||
return composeConfigs([...configs, preview]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: preview is used in composeConfigs even when csfFactoryPreview is found, which could cause conflicts. Should only include preview in non-factory case. |
||
}`; | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-shadow | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,21 @@ import { global } from '@storybook/global'; | |
|
||
import { importFn } from '{{storiesFilename}}'; | ||
|
||
const getProjectAnnotations = () => composeConfigs(['{{previewAnnotations_requires}}']); | ||
const getProjectAnnotations = () => { | ||
const previewAnnotations = ['{{previewAnnotations_requires}}']; | ||
// the last one in this array is the user preview | ||
const preview = previewAnnotations[previewAnnotations.length - 1]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Assuming the last preview is the user preview could be fragile if the array order changes. Consider adding a more explicit way to identify the user preview. |
||
|
||
const csfFactoryPreview = Object.values(preview).find((module) => { | ||
return 'isCSFFactoryPreview' in module; | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: Object.values() on preview could be undefined if preview is not an object. Add type checking or error handling. |
||
|
||
if (csfFactoryPreview) { | ||
return csfFactoryPreview.annotations; | ||
} | ||
|
||
return composeConfigs(previewAnnotations); | ||
}; | ||
|
||
const channel = createBrowserChannel({ page: 'preview' }); | ||
addons.setChannel(channel); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Type casting to 'any' should be avoided. Consider creating proper type definitions for CSF factories to maintain type safety.