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 invalid URL when using relative URL and searchParams #236

Closed
uniblackfire opened this issue Feb 12, 2020 · 8 comments · Fixed by #271
Closed

TypeError invalid URL when using relative URL and searchParams #236

uniblackfire opened this issue Feb 12, 2020 · 8 comments · Fixed by #271

Comments

@uniblackfire
Copy link

hello!
when using ky with @nuxt/http but encountered an error.

    const response = await $http.$get('list', {
      prefixUrl: '/api,
      searchParams: {
        page: 1,
      },
    });

the code above triggers an error:

UnhandledPromiseRejectionWarning: TypeError [ERR_INVALID_URL]: Invalid URL: /api/list
    at onParseError (internal/url.js:243:9)
    at new URL (internal/url.js:319:5)
    at new Ky (node_modules/ky/umd.js:246:0)

and then i check node_modules/ky/umd.js and MDN URL()

if (this._options.searchParams) {
  const url = new URL(this.request.url);
  url.search = new URLSearchParams(this._options.searchParams);
  this.request = new globals.Request(new globals.Request(url, this.request), this._options);
}
@sholladay
Copy link
Collaborator

Duplicate of #228

@sholladay sholladay marked this as a duplicate of #228 Feb 12, 2020
@sholladay
Copy link
Collaborator

Unfortunately, due to limitations in the WHATWG URL API, this.request.url needs to be an absolute URL, otherwise the new URL() constructor call will fail. Generally speaking, that's not a problem because, in most environments, new Request() will have already resolved it to an absolute URL by the time we reach this part of the code. However, if you are using Ky on the server or in an environment where Request is polyfilled, then that implementation may not resolve the URL if it doesn't have a base URL to use.

We try to limit the impact of this problem by only using new URL() where necessary. However, if your Request implementation does not resolve against a base URL, then you cannot use Ky with both a relative URL (prefixUrl in your case) AND the searchParams option together.

See #228 (comment) for more details and thoughts on how we could fix this. I am happy to review any PR related to this problem, however there probably won't be an elegant solution without help from the folks at WHATWG. I would encourage anyone who runs into this to open an issue over on https://github.com/whatwg/url/issues. We basically need utilities for parsing and manipulating relative URLs. They can be faked in userland, but they should be standardized.

Until there is a proper fix within Ky, my suggestion to you is to do something like this:

const baseUrl = isRunningOnServer ? `https://${host}` : '';
const response = await $http.$get('list', {
    prefixUrl: `${baseUrl}/api`,
    searchParams: {
        page: 1,
    }
});

For the host variable, you could use the value of the Host header, if you are executing in the context of a request handler on a server. Otherwise, just use the public hostname + port of your server.

@uniblackfire
Copy link
Author

@sholladay oh! thank you for your solution!
actually, my code is executed in life cycle method asyncData of Nuxt.js and it is running in server.
thanks again for your help!

@Airkro
Copy link

Airkro commented May 9, 2020

Not only on sever. Got same problem in iOS 10.x.

@sholladay
Copy link
Collaborator

@Airkro I don't have an iOS device on hand to test with. Could you provide a video or other evidence of this to help debug it? I'll test with BriwserStack when I get a moment.

@Airkro
Copy link

Airkro commented May 9, 2020

@sholladay

I don't have an iOS device either. I borrow that for a moment from my workmate.

I used https://polyfill.io/v3/polyfill.min.js?features=fetch,AbortController,URL,URLSearchParams to make ky work on iOS 10.x

Then:

  ky.get('sample', {
-   prefixUrl: '/api/'
+   prefixUrl: window.location.origin + '/api/'
  })

But it is unnecessary on iOS 13.x

@sholladay
Copy link
Collaborator

sholladay commented May 9, 2020

Interesting, good to know. Unfortunately, I'm not sure we'll be able to do anything about it if it's a bug with the environment. We used to resolve the request URL against the document's base URL, which is how I would fix it, but that caused problems in some environments.

I think your workaround is good. However, I recommend using new URL(document.baseURI).origin instead of window.location.origin, if your target environment can support it.

@sholladay sholladay changed the title TypeError invalid URL when using option prefixUrl and searchParams TypeError invalid URL when using relative URL and searchParams Jul 15, 2020
@sholladay
Copy link
Collaborator

I just opened PR #271 to fix this. It won't magically make things work if the fetch() implementation doesn't know what to do with relative URLs like /foo, but it ensures that Ky passes through such relative URLs without attempting to absolutely resolve them first. In most cases, fetch() should be able to do the rest of the work for us in that regard.

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 a pull request may close this issue.

3 participants