-
-
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
Core: Add polyfill for fetch #7401
Conversation
This pull request is automatically deployed with Now. Latest deployment for this branch: https://monorepo-git-fork-adamdoyle-7263-add-fetch-polyfill.storybook.now.sh |
I saw this warning earlier this week as well! If I may, I'm going to play devil's advocate for a second, if it's okay:
To that I'd pose the following alternatives:
|
yeah If we can make these polyfills optional that would be a HUGE plus. |
Do you think you could @adamdoyle |
Both if (!self.fetch) {
self.fetch = /* ... polyfill */;
} So they're already "conditional" in that sense. Unless I'm mistaken, anything beyond that would require either (1) a compile-time flag to exclude it from the webpack output, or (2) a dynamic import. I can switch it to |
Yep, dynamic import is where my mind was at, especially since Webpack has first class support for it. |
I could throw together something like this (they're using parcel, but same idea). I just want to make sure you (@emilio-martinez) and @ndelangen are on board with the general idea before I put together a PR. |
Yea the imperative imports |
Hello All, |
@bhargavshah I think we're waiting on a lighter polyfill, though tbh I'm fine with the current fix (cc @adamdoyle). As for releasing, we usually release it on |
@ndelangen @emilio-martinez updated to use dynamic import and switched to unfetch, as requested should be ready for review |
It would be awesome to get this shipped! My team hasn't been able to test components in IE for a while 😭 |
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.
AWESOME
Issue: #7263
What I did
@storybook/core
dependency on fetch-wg polyfill for fetchlib/core/src/server/common/polyfill.js
How to test
Reproduce issue:
Failed to fetch latest version from server: TypeError: Object expected
Test the fix:
XMLHttpRequest for https://storybook.js.org/versions.json?current=5.2.0-alpha.42 required Cross Origin Resource Sharing (CORS).
Note that the version check stores a timestamp for the latest successful attempt in local storage and won't check again until 24 hours past that timestamp. You can work around that in IE11 by invoking
localStorage.clear()
on the Javascript console.Also note that in the latest version there also exists issue #7388 that also prevents IE from loading. Until that issue is resolved, IE11 still won't completely load. The author of #7263 was using
v4.1.0
, which didn't yet have this additional IE issue.