Portable stories: Use internal Storybook renderers mechanism #28766
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 8f39a19. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
yannbf
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
27 file(s) reviewed, 25 comment(s)
Edit PR Review Bot Settings
| globalProjectAnnotations, | ||
| projectAnnotations ?? {}, | ||
| ]) | ||
| composeConfigs([defaultConfig ?? {}, globalProjectAnnotations, projectAnnotations ?? {}]) |
There was a problem hiding this comment.
style: Ensure defaultConfig is correctly populated to avoid unexpected behavior.
| @@ -130,7 +113,7 @@ export function composeStory<TRenderer extends Renderer = Renderer, TArgs extend | |||
| loaded: {}, | |||
| abortSignal: new AbortController().signal, | |||
| step: (label, play) => story.runStep(label, play, context), | |||
There was a problem hiding this comment.
style: Setting canvasElement to null! might cause runtime errors if not properly handled.
| const context = initializeContext(); | ||
| context.canvasElement ??= globalThis?.document?.body; |
There was a problem hiding this comment.
style: Check if globalThis.document.body is always available in all environments.
| @@ -325,13 +305,24 @@ export function createPlaywrightTest<TFixture extends { extend: any }>( | |||
|
|
|||
| // TODO At some point this function should live in prepareStory and become the core of StoryRender.render as well. | |||
| // Will make a follow up PR for that | |||
| async function playStory<TRenderer extends Renderer>( | |||
| async function runStory<TRenderer extends Renderer>( | |||
There was a problem hiding this comment.
logic: Consider extracting the container creation logic to a helper function for better readability.
| @@ -235,42 +235,6 @@ export class MountMustBeDestructuredError extends StorybookError { | |||
| } | |||
There was a problem hiding this comment.
style: Ensure the internal renderToCanvas method is robust and can handle all scenarios previously managed by testingLibraryRender.
| @@ -1,6 +1,5 @@ | |||
| ```tsx filename="setupTest.ts" renderer="react" language="ts" | |||
| import { beforeAll } from '@jest/globals'; | |||
| import { render as testingLibraryRender } from '@testing-library/react'; | |||
| import { setProjectAnnotations } from '@storybook/react'; | |||
There was a problem hiding this comment.
style: Ensure that the internal renderToCanvas method provides equivalent functionality to testingLibraryRender.
| @@ -9,8 +8,6 @@ import * as previewAnnotations from './.storybook/preview'; | |||
| const annotations = setProjectAnnotations([ | |||
| previewAnnotations, | |||
| addonAnnotations, | |||
| // You MUST provide this option to use portable stories with vitest | |||
| { testingLibraryRender }, | |||
| ]); | |||
There was a problem hiding this comment.
check: Verify that all necessary annotations are included to avoid missing configurations.
| @@ -19,7 +16,6 @@ beforeAll(annotations.beforeAll); | |||
|
|
|||
| ```tsx filename="setupTest.ts" renderer="vue" language="ts" | |||
| import { beforeAll } from '@jest/globals'; | |||
| import { render as testingLibraryRender } from '@testing-library/vue'; | |||
| import { setProjectAnnotations } from '@storybook/vue3'; | |||
There was a problem hiding this comment.
style: Ensure that the internal renderToCanvas method provides equivalent functionality to testingLibraryRender.
| @@ -28,8 +24,6 @@ import * as previewAnnotations from './.storybook/preview'; | |||
| const annotations = setProjectAnnotations([ | |||
| previewAnnotations, | |||
| addonAnnotations, | |||
| // You MUST provide this option to use portable stories with vitest | |||
| { testingLibraryRender }, | |||
| ]); | |||
There was a problem hiding this comment.
check: Verify that all necessary annotations are included to avoid missing configurations.
| const annotations = setProjectAnnotations([ | ||
| previewAnnotations, | ||
| addonAnnotations, | ||
| // You MUST provide this option to use portable stories with vitest | ||
| { testingLibraryRender }, | ||
| ]); |
There was a problem hiding this comment.
style: Ensure that removing testingLibraryRender does not affect existing tests or configurations.
… into kasper/render-agnostic
… into kasper/render-agnostic
| @@ -152,6 +173,9 @@ exports[`Renders NewStory story 1`] = ` | |||
| </div> | |||
| <!----> | |||
| <!----> | |||
| <!----> | |||
| <!----> | |||
| <!----> | |||
There was a problem hiding this comment.
any idea on what's going on here?
There was a problem hiding this comment.
Yes, those are coming from PreviewRender that was first not used in the testing library setup
| ); | ||
| }, | ||
| play: async ({ canvasElement, step }) => { | ||
| console.log('start of play function'); |
There was a problem hiding this comment.
I don't know, I think you added them, I copied it from the test sandbox
| @@ -14,7 +14,7 @@ import { setProjectAnnotations, composeStories, composeStory } from '..'; | |||
| import type { Button } from './Button'; | |||
| import * as stories from './Button.stories'; | |||
|
|
|||
| setProjectAnnotations([{ testingLibraryRender: render }]); | |||
| setProjectAnnotations([]); | |||
There was a problem hiding this comment.
setProjectAnnotations still register the react annoations. So I need to call it. And TS accepts only accepts an empty array, we probably should change that though?
...ks/portable-stories-kitchen-sink/nextjs/stories/__snapshots__/portable-stories.test.tsx.snap
Show resolved
Hide resolved
test-storybooks/portable-stories-kitchen-sink/react/.storybook/preview.tsx
Show resolved
Hide resolved
Closes #28685
What I did
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/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 PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>Greptile Summary
This pull request introduces an internal
renderToCanvasmethod as a fallback whentestingLibraryRenderis not specified, along with several related changes.code/core/src/preview-api/modules/store/csf/portable-stories.ts: Removed fallback logic forrenderToCanvasand introduced a newrunfunction for story execution.code/core/src/preview-errors.ts: RemovedTestingLibraryMustBeConfiguredErrorclass to simplify error handling.code/core/src/types/modules/composedStory.ts: Added optionalplayand mandatoryrunfunctions toComposedStoryFntype.code/lib/react-dom-shim/src/preventActChecks.tsx: AddedpreventActChecksfunction to temporarily disableIS_REACT_ACT_ENVIRONMENTflag.code/renderers/react/src/portable-stories.tsx: Introduced conditional check to use internalrenderToCanvasmethod iftestingLibraryRenderis not specified.