-
-
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
Add headers to enable SharedArrayBuffer in stories #16970
Merged
ndelangen
merged 4 commits into
storybookjs:next
from
darthtrevino:feature/shared_array_buffer_headers
Mar 22, 2022
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@darthtrevino this caused me havoc on friday.
This exact example or any https:image file here got hit with cors.
This also got hit with cors
<script src="https://cdn.tailwindcss.com/"></script>Error in chrome and firefox -> "Failed to load resource: net::ERR_BLOCKED_BY_RESPONSE.NotSameOriginAfterDefaultedToSameOriginByCoep"
I traced it to my yarn lock and then traced it from alpha 45 -> to alpha 50.
When you get a chance can you check create a theme kickstart (https://storybook.js.org/docs/react/configure/theming) or even attempt to load any script "https" in preview.html ->
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.
Ok so doing more research this change forces all img and src tags to add cross-origin. Research -> Chrome Coop-Coep, The video and this section (Note: Ask the owner part)
TLDR:
Ask the owner of the resource to support either CORS or CORP.
We currently do not have access to img tag for the brandImage so thats one thing but I would assume if you added crossorigin & the 3rd party/external server supported cors it would work.
However, for the tailwind url or any server that does not returns cors, the browser will block it in this 'cross-origin isolated' state.
NOTE: In the "Response" below this 3rd party resource does not return cors, so although the local dev server sets allow-access-control. Chrome will block this request.
Response
Request
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.
If my understanding is right maybe you can add COOP and COEP as a flag for story book instead, just like ssl is a flag. That way if you need shared array buffer, you will have to deal with the reality that any external resource that does not support CORS will not work.. WHICH is working as expected. However IMO I find this to be more of an edge case and therefore fits better as a flag with some caveat text around it.