Skip to content
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

Composition: Fix various CORS issues. #21682

Merged
merged 2 commits into from
Mar 20, 2023
Merged

Conversation

tmeasday
Copy link
Member

@tmeasday tmeasday commented Mar 20, 2023

Closes #21500 Closes #21501

image

image

image

What I did

It seems that fetch() can throw directly for various CORS related errors, rather than returning a response with { ok: false }.

As we make 3 requests (index.json, stories.json, metadata.json) and it is common for servers to fail CORS when 404-ing, there's a good chance of hitting one of these (for instance Chakra does it on metadata.json, Chromatic does it on index.json <- 🐛 ).

So I:

  • wrote some tests demonstrating the behaviour
  • fixed the handling (it was a bit wonky anyway)

How to test

I used these three refs:

  refs: {
    foo: {
      title: 'Chakra',
      url:'https://storybook.chakra-ui.com/'
    },
    bar: {
      title: 'CH',
      url: 'https://59c59bd0183bd100364e1d57-jjsanukdmo.chromatic.com/'
    },
    baz: {
      title: 'local',
      url: 'http://localhost:8080/'
    }
  }

For the second you need access to the private repo and for the third you need to be running npx http-server storybook-static --cors

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

@ndelangen ndelangen merged commit 5658390 into next Mar 20, 2023
@ndelangen ndelangen deleted the tom/21500-fix-cors-issues branch March 20, 2023 13:08
@yarindeoh
Copy link

The fix was released in 7.0.0-rc.5 ?

@tmeasday
Copy link
Member Author

7.0.0-rc.6 it looks like @yarindeoh:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Composition - CORS issue with auth [Bug]: composition - CORS issues loading stories/index .json
3 participants