-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Core: Disable interactions debugger on composed stories to avoid cross-origin error #31685
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
Conversation
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.
2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
|
View your CI Pipeline Execution ↗ for commit fd2e026.
☁️ Nx Cloud last updated this comment at |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 49 | 49 | 0 |
| Self size | 31.40 MB | 31.38 MB | 🎉 -22 KB 🎉 |
| Dependency size | 17.41 MB | 17.41 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
sb
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 50 | 50 | 0 |
| Self size | 1 KB | 1 KB | 0 B |
| Dependency size | 48.81 MB | 48.79 MB | 🎉 -22 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 324 | 324 | 0 |
| Self size | 244 KB | 244 KB | 0 B |
| Dependency size | 97.79 MB | 97.81 MB | 🚨 +18 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 267 | 267 | 0 |
| Self size | 31 KB | 31 KB | 0 B |
| Dependency size | 82.19 MB | 82.21 MB | 🚨 +22 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
|
Why disable it instead of fixing it? The data could be passed from the child to the parent with https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage |
|
|
||
| loadParentWindowState = () => { | ||
| try { | ||
| this.state = global.window?.parent?.__STORYBOOK_ADDON_INTERACTIONS_INSTRUMENTER_STATE__ || {}; |
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.
This should be changed to use window.postMessage() to enable safe cross-origin communication.
|
I have done similar things to fix issues syncing toolbar state across composed storybook instances. I have this in the preview.tsx file of the parent instance of my component library: // Propagates global updates to composed storybook instances
window.addEventListener('message', (event) => {
const message = JSON.parse(event.data?.trim?.() || '{}');
if (message.key !== 'storybook-channel') return;
if (message.event?.type !== 'updateGlobals') return;
window.top?.document.querySelectorAll('iframe').forEach((iframe) => {
if (iframe.contentWindow === window) return;
iframe.contentWindow?.postMessage(event.data, '*');
});
}); |
|
@shannonmoeller Unfortunately we can't use postMessage because as far as I know, its serialization method will lose object references. Instrumenter state contains a property |
| get __STORYBOOK_ADDON_INTERACTIONS_INSTRUMENTER_STATE__() { | ||
| throw new Error('Blocked'); | ||
| }, | ||
| set __STORYBOOK_ADDON_INTERACTIONS_INSTRUMENTER_STATE__(_: unknown) {}, |
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.
Long term, would be great to stop having to do this somehow.
Accessing global.window.parent should never be done.
Surely this can be done using the channel?
Till then this looks like an improvement to me.
…tions-state Core: Disable interactions debugger on composed stories to avoid cross-origin error (cherry picked from commit 909f244)
Closes #31650
What I did
Gracefully handle cross-origin errors when accessing a composed Storybook with interactions, by catching the error and disabling debugger controls.
The instrumenter attempts to retain its state when reloading the preview iframe by storing its state on the parent window. This is not allowed on composed Storybooks due to the same-origin policy. In most cases this is not a problem because the iframe will only be reloaded if the story can't be remounted within 3 'ticks' (see
StoryRender.teardown()). If this scenario does happen, the instrumenter will lose its state. This is an edge case we can't easily resolve, because the state we're trying to preserve relies on object references and therefore can't be serialized (e.g. for sessionStorage or postMessage/channel).Because we cannot predict when the iframe needs a full reload, I've decided to assume the worst and disable the debugger controls for composed Storybooks altogether.
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 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
Fixes cross-origin security issues in Storybook's interactions panel when accessing composed stories by gracefully handling parent window state access restrictions.
code/core/src/instrumenter/instrumenter.tsto handle cross-origin limitationsWindowinterface incode/core/src/instrumenter/typings.d.tsto properly declare global state variables