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

Support URL object for fetch and fix getBaseUrl #166

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

akkadaska
Copy link

A solution for #140.

Fixed request.js to support URL object for fetch.

  • Changed to convert URL object to string when the input variable is an instance of URL object.

Fixed getBaseUrl() function in url.js to support fetch URL that has no host for any environment.

  • Changed to return an url whose origin is the same as the url where the storybook is hosted (= location.href) if the url has no host.
  • To test a function that refers to window.location.href, set Jest test environment to jsdom and add mock of window.location.href``.

Note : new URL(url, base) returns url without changing anything if it has host.
e.g. new URL('https://a.com/foo/bar', 'https://b.com') -> https://a.com/foo/bar

@nutboltu
Copy link
Collaborator

@akkadaska can you resolve your merge conflicts?

@netlify
Copy link

netlify bot commented Jul 29, 2023

Deploy Preview for storybook-addon-mock ready!

Name Link
🔨 Latest commit 9fe5c01
🔍 Latest deploy log https://app.netlify.com/sites/storybook-addon-mock/deploys/64c4760ab7f057000885d4e9
😎 Deploy Preview https://deploy-preview-166--storybook-addon-mock.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@akkadaska
Copy link
Author

@nutboltu
Merge conflicts have been successfully resolved. However, tests are failing, which seems to be a pre-existing issue with the master branch.

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

Successfully merging this pull request may close these issues.

2 participants