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

TypeError relative URL after upgrading to 0.5.0 #58

Closed
peterbe opened this issue Nov 4, 2018 · 3 comments · Fixed by #59
Closed

TypeError relative URL after upgrading to 0.5.0 #58

peterbe opened this issue Nov 4, 2018 · 3 comments · Fixed by #59
Labels
bug Something isn't working

Comments

@peterbe
Copy link

peterbe commented Nov 4, 2018

I've got this code:

  fetchHomeCards = async () => {
    this.setState({ loading: true });
    const response = await ky("/api/cards/");
    if (response.ok) {
...

it used to work just fine in 0.4.1, but busted now.

In Firefox I get TypeError: /api/cards/ is not a valid URL. on this line:

var url = new _globalThis.URL(this._options.prefixUrl + this._input);

In Chrome I get a lot more helpful error:

index.js:181 Uncaught (in promise) TypeError: Failed to construct 'URL': Invalid URL
    at new Ky (index.js:181)
    at ky (index.js:518)
    at CardsContainer._callee$ (State.js:15)
    at tryCatch (runtime.js:62)
    at Generator.invoke [as _invoke] (runtime.js:288)
    at Generator.prototype.(:3000/anonymous function) [as next] (http://localhost:3000/static/js/1.chunk.js:941:21)
    at asyncGeneratorStep (asyncToGenerator.js:3)
    at _next (asyncToGenerator.js:25)
    at asyncToGenerator.js:32
    at new Promise (<anonymous>)
    at CardsContainer.fetchHomeCards (asyncToGenerator.js:21)
    at Home._callee$ (Home.js:9)
    at tryCatch (runtime.js:62)
    at Generator.invoke [as _invoke] (runtime.js:288)
    at Generator.prototype.(:3000/anonymous function) [as next] (http://localhost:3000/static/js/1.chunk.js:941:21)
    at asyncGeneratorStep (asyncToGenerator.js:3)
    at _next (asyncToGenerator.js:25)
    at asyncToGenerator.js:32
    at new Promise (<anonymous>)
    at Home.<anonymous> (asyncToGenerator.js:21)
    at Home.componentDidMount (Home.js:12)
    at commitLifeCycles (react-dom.development.js:15986)
    at commitAllLifeCycles (react-dom.development.js:17388)
    at HTMLUnknownElement.callCallback (react-dom.development.js:147)
    at Object.invokeGuardedCallbackDev (react-dom.development.js:196)
    at invokeGuardedCallback (react-dom.development.js:250)
    at commitRoot (react-dom.development.js:17549)
    at completeRoot (react-dom.development.js:19002)
    at performWorkOnRoot (react-dom.development.js:18924)
    at performWork (react-dom.development.js:18826)
    at performSyncWork (react-dom.development.js:18799)
    at requestWork (react-dom.development.js:18676)
    at scheduleWork (react-dom.development.js:18480)
    at scheduleRootUpdate (react-dom.development.js:19186)
    at updateContainerAtExpirationTime (react-dom.development.js:19212)
    at updateContainer (react-dom.development.js:19280)
    at ReactRoot.push../node_modules/react-dom/cjs/react-dom.development.js.ReactRoot.render (react-dom.development.js:19559)
    at react-dom.development.js:19713
    at unbatchedUpdates (react-dom.development.js:19064)
    at legacyRenderSubtreeIntoContainer (react-dom.development.js:19709)
    at Object.render (react-dom.development.js:19776)
    at Module../src/index.js (index.js:8)
    at __webpack_require__ (bootstrap:782)
    at fn (bootstrap:150)
    at Object.0 (serviceWorker.js:131)
    at __webpack_require__ (bootstrap:782)
    at checkDeferredModules (bootstrap:45)
    at Array.webpackJsonpCallback [as push] (bootstrap:32)
    at main.chunk.js:1
@peterbe
Copy link
Author

peterbe commented Nov 4, 2018

Downgrading to 0.4.1 solved it.

@sindresorhus
Copy link
Owner

This regression was introduced in a7c71b5. We needed to convert the string URL into an URL instance to be able to add search params. Since URL requires full URLs, it throws.

@sholladay Do you know what fetch() resolves against when it gets foo and /foo URLs? We need to imitate that for base in the new URL() call.

// @itaisteinherz

@sindresorhus sindresorhus added the bug Something isn't working label Nov 4, 2018
@sholladay
Copy link
Collaborator

Oy sorry about that. I wanted to write a test for that case, but we didn't have Cypress set up at the time and couldn't get it to work with node-fetch. The answer is document.baseURI. I will do a PR in a few hours if no one gets to it by then.

sindresorhus pushed a commit that referenced this issue Nov 6, 2018
Fixes #58

There was a regression introduced in a7c71b5 which broke the ability for `input` to be a relative URL like `foo` or `/foo`. The root cause is that [`window.URL`](https://developer.mozilla.org/en-US/docs/Web/API/URL) unfortunately does not support relative URLs without an explicit and absolute `base` argument. This fixes that and adds a test for it. Now that we have Cypress set up, we can properly test anything related to relative URLs.

The goal is to follow the URL resolution rules the same way that `fetch` does. So we resolve `input` against [`document.baseURI`](https://developer.mozilla.org/en-US/docs/Web/API/Node/baseURI#The_base_URL_of_a_document).
savvy-bit pushed a commit to savvy-bit/ky that referenced this issue Mar 24, 2024
Fixes sindresorhus/ky#58

There was a regression introduced in sindresorhus/ky@a7c71b5 which broke the ability for `input` to be a relative URL like `foo` or `/foo`. The root cause is that [`window.URL`](https://developer.mozilla.org/en-US/docs/Web/API/URL) unfortunately does not support relative URLs without an explicit and absolute `base` argument. This fixes that and adds a test for it. Now that we have Cypress set up, we can properly test anything related to relative URLs.

The goal is to follow the URL resolution rules the same way that `fetch` does. So we resolve `input` against [`document.baseURI`](https://developer.mozilla.org/en-US/docs/Web/API/Node/baseURI#The_base_URL_of_a_document).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants