refactor: Replace `url.parse` with `new URL()` #701

xxczaki · 2019-12-02T18:55:18Z

I think this is the last thing we need to do in order to release the 3.x alpha release.

Replacing the url.parse API here doesn't break our tests:
https://github.com/bitinn/node-fetch/blob/3b452b8e1f280142d78d5a33abf945d77c25ea59/src/request.js#L51

However, trying to use the new WHATWG API here:
https://github.com/bitinn/node-fetch/blob/3b452b8e1f280142d78d5a33abf945d77c25ea59/src/request.js#L54

throws some errors:

> [email protected] test /home/akepinski/dev/node-fetch
> cross-env BABEL_ENV=test mocha --require @babel/register --throw-deprecation test/test.js



  node-fetch
    ✓ should return a promise
    1) should return a promise
(node:11807) UnhandledPromiseRejectionWarning: FetchError: request to http://localhost:30001/hello failed, reason: connect ECONNREFUSED 127.0.0.1:80
    at ClientRequest.<anonymous> (/home/akepinski/dev/node-fetch/src/index.js:113:11)
    at ClientRequest.emit (events.js:210:5)
    at Socket.socketErrorListener (_http_client.js:406:9)
    at Socket.emit (events.js:210:5)
    at emitErrorNT (internal/streams/destroy.js:92:8)
    at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
    at processTicksAndRejections (internal/process/task_queues.js:80:21)
(node:11807) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)


  1 passing (35ms)
  1 failing

  1) node-fetch
       should return a promise:
     Uncaught DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
      at emitDeprecationWarning (internal/process/promises.js:153:11)
      at processPromiseRejections (internal/process/promises.js:205:13)
      at processTicksAndRejections (internal/process/task_queues.js:94:32)



npm ERR! Test failed.  See above for more details.

Trying to use the new API here also breaks our tests, but with more precise errors:

175 passing (2s)
  29 failing

  1) node-fetch
       should follow redirect code 301:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  2) node-fetch
       should follow redirect code 302:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  3) node-fetch
       should follow redirect code 303:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  4) node-fetch
       should follow redirect code 307:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  5) node-fetch
       should follow redirect code 308:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  6) node-fetch
       should follow redirect chain:
     FetchError: request to http://localhost:30001/redirect/301 failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  7) node-fetch
       should follow POST request redirect code 301 with GET:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  8) node-fetch
       should follow PATCH request redirect code 301 with PATCH:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  9) node-fetch
       should follow POST request redirect code 302 with GET:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  10) node-fetch
       should follow PATCH request redirect code 302 with PATCH:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  11) node-fetch
       should follow redirect code 303 with GET:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  12) node-fetch
       should follow PATCH request redirect code 307 with PATCH:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  13) node-fetch
       should obey maximum redirect, reject case:

      AssertionError: expected { type: 'system',
  name: 'FetchError',
  errno: 'ECONNREFUSED',
  code: 'ECONNREFUSED',
  erroredSysCall: [Error: connect ECONNREFUSED 127.0.0.1:80] } to have property 'type' of 'max-redirect', but got 'system'
      + expected - actual

      -system
      +max-redirect
      
      at /home/akepinski/dev/node-fetch/node_modules/chai-as-promised/lib/chai-as-promised.js:302:22
      at processTicksAndRejections (internal/process/task_queues.js:93:5)

  14) node-fetch
       should obey redirect chain, resolve case:
     FetchError: request to http://localhost:30001/redirect/301 failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  15) node-fetch
       should follow redirect code 301 and keep existing headers:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  16) node-fetch
       should set redirected property on response when redirect:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  17) node-fetch
       should allow custom timeout on redirected requests:

      AssertionError: expected { type: 'system',
  name: 'FetchError',
  errno: 'ECONNREFUSED',
  code: 'ECONNREFUSED',
  erroredSysCall: [Error: connect ECONNREFUSED 127.0.0.1:80] } to have property 'type' of 'request-timeout', but got 'system'
      + expected - actual

      -system
      +request-timeout
      
      at /home/akepinski/dev/node-fetch/node_modules/chai-as-promised/lib/chai-as-promised.js:302:22
      at processTicksAndRejections (internal/process/task_queues.js:93:5)

  18) node-fetch
       should allow redirects to be aborted:

      AssertionError: expected { type: 'system',
  name: 'FetchError',
  errno: 'ECONNREFUSED',
  code: 'ECONNREFUSED',
  erroredSysCall: [Error: connect ECONNREFUSED 127.0.0.1:80] } to have property 'name' of 'AbortError', but got 'FetchError'
      + expected - actual

      -FetchError
      +AbortError
      
      at /home/akepinski/dev/node-fetch/node_modules/chai-as-promised/lib/chai-as-promised.js:302:22
      at processTicksAndRejections (internal/process/task_queues.js:93:5)

  19) node-fetch
       should allow redirected response body to be aborted:

      AssertionError: expected { type: 'system',
  name: 'FetchError',
  errno: 'ECONNREFUSED',
  code: 'ECONNREFUSED',
  erroredSysCall: [Error: connect ECONNREFUSED 127.0.0.1:80] } to have property 'name' of 'AbortError', but got 'FetchError'
      + expected - actual

      -FetchError
      +AbortError
      
      at /home/akepinski/dev/node-fetch/node_modules/chai-as-promised/lib/chai-as-promised.js:302:22
      at processTicksAndRejections (internal/process/task_queues.js:93:5)

  20) node-fetch
       should remove internal AbortSignal event listener after request and response complete without aborting:
     AssertionError: expected promise to be fulfilled but it was rejected with 'FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80'
  

  21) node-fetch
       should support fetch with Request instance:
     FetchError: request to http://localhost:30001/hello failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  22) node-fetch
       should support fetch with Node.js URL object:
     FetchError: request to http://localhost:30001/hello failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  23) node-fetch
       should support fetch with WHATWG URL object:
     FetchError: request to http://localhost:30001/hello failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  24) node-fetch
       should support overwrite Request instance:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  25) node-fetch
       supports supplying a lookup function to the agent:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  26) node-fetch
       supports supplying a famliy option to the agent:
     FetchError: request to http://localhost:30001/inspect failed, reason: connect ECONNREFUSED 127.0.0.1:80
      at ClientRequest.<anonymous> (src/index.js:113:11)
      at Socket.socketErrorListener (_http_client.js:406:9)
      at emitErrorNT (internal/streams/destroy.js:92:8)
      at emitErrorAndCloseNT (internal/streams/destroy.js:60:3)
      at processTicksAndRejections (internal/process/task_queues.js:80:21)

  27) Request
       should override signal on derived Request instances:
     TypeError [ERR_INVALID_URL]: Invalid URL: test
      at onParseError (internal/url.js:243:9)
      at new URL (internal/url.js:319:5)
      at new Request (src/request.js:59:16)
      at Context.<anonymous> (test/test.js:2617:26)
      at processImmediate (internal/timers.js:439:21)

  28) Request
       should allow removing signal on derived Request instances:
     TypeError [ERR_INVALID_URL]: Invalid URL: test
      at onParseError (internal/url.js:243:9)
      at new URL (internal/url.js:319:5)
      at new Request (src/request.js:59:16)
      at Context.<anonymous> (test/test.js:2629:26)
      at processImmediate (internal/timers.js:439:21)

  29) external encoding
       data uri
         should accept data uri of plain text:

      AssertionError: expected '' to equal 'Hello World!'
      + expected - actual

      +Hello World!
      
      at equal (test/test.js:2835:44)



npm ERR! Test failed.  See above for more details.

bitinn · 2019-12-06T05:09:01Z

Sorry for the late reply, I will look into it when available.

But the tests appear to fail due to completely unrelated issues. Can we please get the CI back to a working state before proceeding to troubleshoot this issue?

https://travis-ci.org/bitinn/node-fetch/jobs/619760485

Richienb · 2019-12-13T09:00:58Z

@bitinn Tests are fixed in #703

bitinn · 2019-12-15T10:07:45Z

@xxczaki @Richienb I have re-ran the tests, the build is now passing (I don't know how should accept data uri of plain text test fail for the push but the PR passes), please remove utf8 package dependency and we can merge this 👍

Richienb · 2019-12-15T10:09:20Z

@bitinn We haven't migrated parseUrl to new URL for all of the instances yet...

bitinn · 2019-12-15T11:23:49Z

@xxczaki @Richienb

I have figured out exactly why this is happening:

When we parse the URL object in request.js, we use ... spread syntax to read its properties, but this doesn't work with new URL() object (its a completely different URL object from the ones created by url.parse()).

Then in index.js we check for options.protocol in order to determine the request protocol, which defaults to https when protocol property is missing. Of course a request to http://localhost:30001/hello using https.request will result in ECONNREFUSED

I have branch with a fix, but it's not complete, some of the tests require new Request() and new Response() to support any URL input, which new URL can't do without a base parameter. So I guess we either need to prevent these edge cases. Or use a default base when URL is not absolute.

I am too busy to fix these so please follow up on this problem :)

https://github.com/bitinn/node-fetch/tree/new-url-parser

Richienb · 2019-12-15T11:25:22Z

@bitinn It's currently late at night for me at the moment (UTC+12) so I'll take this tomorrow.

bitinn · 2019-12-15T11:29:02Z

Take your time, we are not in a rush here :)

Also new URL() relative url parsing issue is known, documented by Timothy as always: nodejs/node#12682

xxczaki · 2020-01-07T17:52:43Z

Ok, so I was working with David's branch and here is what I did:

To handle relative URLs I created a parseURL function (inspiration), which is basically a wrapper around new URL. It uses a solution suggested here, which isn't perfect.

Now only 2 tests fail:

  202 passing (2s)
  2 failing

  1) node-fetch
       should reject with error if url is protocol relative:

      AssertionError: expected promise to be rejected with an error including 'Only absolute URLs are supported' but got 'Only HTTP(S) protocols are supported'
      + expected - actual

      -Only HTTP(S) protocols are supported
      +Only absolute URLs are supported
      
  

  2) Request
       should support arbitrary url:

      AssertionError: expected 'relative:///anything' to equal 'anything'
      + expected - actual

      -relative:///anything
      +anything
      
      at Context.equal (test/test.js:2735:22)
      at processImmediate (internal/timers.js:439:21)

I think we might need to release the 3.x version with the deprecated url.parse(), as it looks like new URL() still has some important issues (like the one with relative URLs).

cc @bitinn @Richienb

Richienb · 2020-01-08T00:23:41Z

@xxczaki In that case, we should use a ponyfill for url.parse to future proof.

bitinn · 2020-01-23T04:19:49Z

I have taken a brief look into the errors.

I think the first test error is easy to solve, we just need to change the error message,

https://github.com/node-fetch/node-fetch/blob/3.x/test/test.js#L113-L116

The second error I think is an edge case: node-fetch isn't designed with relative url in mind, due to lack of context. So if you pass Request ctor a relative url, perhaps the right thing is to throw a custom error to let users know (because it's a programming error, and a limit of node-fetch, and it matches the behavior when you pass main fetch() the same relative url).

It should be possible to detect whether the url is absolute or not (perhaps within the parseURL function we introduced here), and once it passes, we will use new URL() with a default base such as relative:///.

In short, I really think we should ship with new URL() instead of url.parse().

xxczaki · 2020-02-19T19:28:45Z

@bitinn I've introduced the detection of whether the URL is absolute or not (regex was taken from here). I've also changed the error message, as you suggested.

Now, should we stop supporting relative URLs and throw a custom error instead? Or maybe we should do something else?

bitinn · 2020-02-20T07:35:25Z

I think we should throw an Error stating that we don’t support relative URL yet. To me this is not possible without the “context”, so even if we workaround it, node-fetch still won’t do the same thing as browsers’ fetch. I think we should also throw if you create a Request or Response manually with relative URL. (From my phone)

…

On Feb 20, 2020, at 3:28, Antoni Kepinski ***@***.***> wrote: @bitinn I've introduced the detection of whether the URL is absolute or not (regex was taken from here). I've also changed the error message, as you suggested. Now, should we stop supporting relative URLs and throw a custom error instead? Or maybe we should do something else? cc @bitinn @Richienb — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

xxczaki · 2020-03-07T20:17:32Z

@bitinn Ok, these tests fail:

1) should override signal on derived Request instances
2) should allow removing signal on derived Request instances
3) should default to null as body
4) should support arbitrary url
5) should support ArrayBuffer as body
6) should support Uint8Array as body
7) should support DataView as body

1, 2 and 4 are related to arbitrary URLs. 3, 5, 6 and 7 do not specify the URL (so the Response class assumes it's an empty string).

bitinn · 2020-03-08T09:23:49Z

@jimmywarting what's your thought on this? obvious we would like to support arbitrary URL but the new URL() just couldn't handle it without some nasty hacks.

jimmywarting · 2020-03-08T12:28:25Z

I also made an attempt to get in the new URL long time ago. but i also stumble upon some failing tests then.

It would be an nice addition to get in the new URL for sure. Even if it meant that we got some new failing test and possible some breaking changes. like throwing an error for constructing request with a relative url. But that's maybe something that we can fix arbitrary URL after this.

bitinn · 2020-03-08T14:59:06Z

@xxczaki

Let's go with no arbitrary url for now?

3, 5, 6 and 7 do not specify the URL (so the Response class assumes it's an empty string).

We can fix these by supplying a placeholder url

1, 2 and 4 are related to arbitrary URLs

If we can use full URL and fix these, do so; otherwise make changes to the test itself to reflect the changes we have made.

And finally, please update UPGRADE doc to reflect this, thx a lot on your efforts to move this PR forward!

xxczaki · 2020-03-08T22:19:51Z

Using this opportunity, I did some small refactoring:

Split tests into different files (though the main one is still huge)
Removed ignored xo rules to improve code quality

…into whatwg-url

bitinn

this mostly LGTM, 2 things:

can you point me to changed tests now that there are refactoring going on, I can't see which tests are changed?
I wrote a comment on about the upgrade guide, I feel it can be explained better.

bitinn · 2020-03-09T02:50:14Z

docs/v3-UPGRADE-GUIDE.md

@@ -67,6 +67,10 @@ We are working towards changing body to become either null or a stream.

 The default user agent has been changed from `node-fetch/1.0 (+https://github.com/node-fetch/node-fetch)` to `node-fetch (+https://github.com/node-fetch/node-fetch)`.

+## Arbitrary URLs are no longer supported
+
+Since in 3.x we are using the WHATWG's `new URL()`, arbitrary URL parsing will fail due to lack of base.


I felt this explanation can be better, something like:

Creating Request/Response objects with relative URLs are no longer supported

We introduce Node.js core's new URL() API in 3.x, it offers better UTF-8 support and is WHATWG URL compatible. The drawback is, given current limit of the API (nodejs/node#12682), it's not possible to support relative URL parsing without hacks.
Due to the lack of a browsing context on Node.js, we opted to drop support for relative URLs on Request/Response object, and it will now throw errors if you do so.
The main fetch() function will support absolute URLs and data url.

Agree. Just updated the upgrade guide 😄

xxczaki · 2020-03-09T11:58:27Z

can you point me to changed tests now that there are refactoring going on, I can't see which tests are changed?

Here are the changes: dec4065

TL;DR:

Deleted one test (should support arbitrary url)
Updated others by supplying a placeholder URL.

Note: In the above commit I used a wrong placeholder URLs (${base}/hello, instead of ${base}hello). Fixed it in ac60e7a.

jimmywarting · 2020-03-09T13:14:09Z

Feels like this PR is getting little out of context and being rather large and doing unrelated changes towards just changing parse with url... more to review.

xxczaki · 2020-03-09T13:51:46Z

@jimmywarting I can remove the refactoring commits and create a separate PR for that. However, since this is probably the last PR before the 3.x release, I wanted to include them in it.

bitinn · 2020-03-09T13:58:09Z

I went through the actual changes on tests and main codebase again, they look fine to me.

Travis CI show the number of tests are correct (206, vs the 207 in v3.x branch currently).

I have one final question though:

Did we test UTF-8 URL support? The actual goal of using new URL()?

jimmywarting · 2020-03-09T15:45:36Z

@jimmywarting I can remove the refactoring commits and create a separate PR for that. However, since this is probably the last PR before the 3.x release, I wanted to include them in it.

you don't have to but just remember that till next time

xxczaki · 2020-03-09T20:45:02Z

@bitinn Will something like this do the trick?

it('URLs are encoded as UTF-8', () => {
	const url = 'http://example.com/möbius';

	fetch(url).then(res => expect(res.url).to.equal('http://example.com/m%C3%B6bius'));
});

bitinn · 2020-03-11T12:46:55Z

@bitinn Will something like this do the trick?

it('URLs are encoded as UTF-8', () => {
	const url = 'http://example.com/möbius';

	fetch(url).then(res => expect(res.url).to.equal('http://example.com/m%C3%B6bius'));
});

I think so, but better to use a local url with our test server script, if it's possible. And of course verify the test fails with current v3 branch, otherwise we are not fixing issues like #245 and #233

xxczaki · 2020-03-12T17:02:56Z

@bitinn Everything seems to be working fine 😄

bitinn · 2020-03-13T04:49:44Z

Then I think it’s time to merge and put v3 out! (whether we should beta it first, can be discussed in the main thread) (From my phone)

…

On Mar 13, 2020, at 1:03, Antoni Kepinski ***@***.***> wrote: @bitinn Everything seems to be working fine 😄 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

* feat: Migrate TypeScript types (#669) * style: Introduce linting via XO * fix: Fix tests * chore!: Drop support for nodejs 4 and 6 * chore: Fix Travis CI yml * Use old Babel (needs migration) * chore: lint everything * chore: Migrate to microbundle * Default response.statusText should be blank (#578) * fix: Use correct AbortionError message Signed-off-by: Richie Bendall <[email protected]> * chore: Use modern @babel/register Signed-off-by: Richie Bendall <[email protected]> * chore: Remove redundant packages Signed-off-by: Richie Bendall <[email protected]> * chore: Readd form-data Signed-off-by: Richie Bendall <[email protected]> * fix: Fix tests and force utf8-encoded urls Signed-off-by: Richie Bendall <[email protected]> * lint index.js * Update devDependencies & ignore `test` directory in linter options * Remove unnecessary eslint-ignore comment * Update the `lint` script to run linter on every file * Remove unused const & unnecessary import * TypeScript: Fix Body.blob() wrong type (DefinitelyTyped/DefinitelyTyped#33721) * chore: Lint as part of the build process * fix: Convert Content-Encoding to lowercase (#672) * fix: Better object checks (#673) * Fix stream piping (#670) * chore: Remove useless check Signed-off-by: Richie Bendall <[email protected]> * style: Fix lint Signed-off-by: Richie Bendall <[email protected]> * style: Fix lint Signed-off-by: Richie Bendall <[email protected]> * refactor: Modernise code Signed-off-by: Richie Bendall <[email protected]> * chore: Ensure all files are properly included Signed-off-by: Richie Bendall <[email protected]> * chore: Update deps and utf8 should be in dependencies Signed-off-by: Richie Bendall <[email protected]> * test: Drop Node v4 from tests Signed-off-by: Richie Bendall <[email protected]> * test: Modernise code Signed-off-by: Richie Bendall <[email protected]> * chore: Move errors to seperate directory Signed-off-by: Richie Bendall <[email protected]> * refactor: Add fetch-blob (#678) * feat: Migrate data uri integration Signed-off-by: Richie Bendall <[email protected]> * Allow setting custom highWaterMark via node-fetch options (#386) (#671) * Expose highWaterMark option to body clone function * Add highWaterMark to responseOptions * Add highWaterMark as node-fetch-only option * a way to silently pass highWaterMark to clone * Chai helper * Server helper * Tests * Remove debug comments * Document highWaterMark option * Add TypeScript types for the new highWaterMark option * feat: Include system error in FetchError if one occurs (#654) * style: Add editorconfig Signed-off-by: Richie Bendall <[email protected]> * chore!: Drop NodeJS v8 Signed-off-by: Richie Bendall <[email protected]> * chore: Remove legacy code for node < 8 Signed-off-by: Richie Bendall <[email protected]> * chore: Use proper checks for ArrayBuffer and AbortError Signed-off-by: Richie Bendall <[email protected]> * chore: Use explicitly set error name in checks Signed-off-by: Richie Bendall <[email protected]> * Propagate size and timeout to cloned response (#664) * Remove --save option as it isn't required anymore (#581) * Propagate size and timeout to cloned response Co-authored-by: Steve Moser <[email protected]> Co-authored-by: Antoni Kepinski <[email protected]> * Update Response types * Update devDependencies * feat: Fallback to blob type (Closes: #607) Signed-off-by: Richie Bendall <[email protected]> * style: Update formatting Signed-off-by: Richie Bendall <[email protected]> * style: Fix linting issues Signed-off-by: Richie Bendall <[email protected]> * docs: Add info on patching the global object * docs: Added non-globalThis polyfill * Replace deprecated `url.resolve` with the new WHATWG URL * Update devDependencies * Format code in examples to use `xo` style * Verify examples with RunKit and edit them if necessary * Add information about TypeScript support * Document the new `highWaterMark` option * Add Discord badge & information about Open Collective * Style change * Edit acknowledgement & add "Team" section * fix table * Format example code to use xo style * chore: v3 release changelog * Add the recommended way to fix `highWaterMark` issues * docs: Add simple Runkit example * fix: Properly set the name of the errors. Signed-off-by: Richie Bendall <[email protected]> * docs: Add AbortError to documented types Signed-off-by: Richie Bendall <[email protected]> * docs: AbortError proper typing parameters Signed-off-by: Richie Bendall <[email protected]> * docs: Add example code for Runkit Signed-off-by: Richie Bendall <[email protected]> * Replace microbundle with @pika/pack (#689) * gitignore the pkg/ directory * Move TypeScript types to the root of the project * Replace microbundle with @pika/pack * chore: Remove @pika/plugin-build-web and revert ./dist output directory Signed-off-by: Richie Bendall <[email protected]> Co-authored-by: Richie Bendall <[email protected]> * fix incorrect statement in changelog * chore: v3.x upgrade guide * Change the Open Collective button * docs: Encode support button as Markdown instead of HTML * chore: Ignore proper directory in xo * Add an "Upgrading" section to readme * Split the upgrade guide into 2 files & add the missing changes about v3.x * style: Lint test and example files Signed-off-by: Richie Bendall <[email protected]> * Move *.md files to the `docs` folder (except README.md) * Update references to files * Split LIMITS.md into 2 files (as of v2.x and v3.x) * chore: Remove logging statement Signed-off-by: Richie Bendall <[email protected]> * style: Fix lint * docs: Correct typings for systemError in FetchError (Fixes #697) * refactor: Replace `encoding` with `fetch-charset-detection`. (#694) * refactor: Replace `encoding` with `fetch-charset-detection`. Signed-off-by: Richie Bendall <[email protected]> * refactor: Move writing to stream back to body.js Signed-off-by: Richie Bendall <[email protected]> * refactor: Only put convertBody in fetch-charset-detection and refactor others. Signed-off-by: Richie Bendall <[email protected]> * test: Readd tests for getTotalBytes and extractContentType Signed-off-by: Richie Bendall <[email protected]> * chore: Revert package.json indention Signed-off-by: Richie Bendall <[email protected]> * chore: Remove optional dependency * docs: Replace code for fetch-charset-detection with documentation. Signed-off-by: Richie Bendall <[email protected]> * chore: Remove iconv-lite * fix: Use default export instead of named export for convertBody Signed-off-by: Richie Bendall <[email protected]> * chore: Remove unneeded installation of fetch-charset-detection in the build * docs: Fix typo * fix: Throw SyntaxError instead of FetchError in case of invalid… (#700) * fix: Throw SyntaxError instead of FetchError in case of invalid JSON Signed-off-by: Richie Bendall <[email protected]> * docs: Add to upgrade guide Signed-off-by: Richie Bendall <[email protected]> * Remove deprecated url.parse from test * Remove deprecated url.parse from server * fix: Proper data uri to buffer conversion (#703) Signed-off-by: Richie Bendall <[email protected]> * chore: Add funding info * fix: Flawed property existence test (#706) Fix a problem where not all prototype methods are copied from the Body via the mixin method due to a failure to properly detect properties in the target. The current code uses the `in` operator, which may return properties lower down the inheritance chain, thus causing them to fail the copy. The new code properly calls the `.hasOwnProperty()` method to make the determination. * fix: Properly handle stream pipeline double-fire Signed-off-by: Richie Bendall <[email protected]> * docs: Fix spelling Signed-off-by: Richie Bendall <[email protected]> * chore: Add `funding` field to package.json (#708) * Fix: Do not set ContentLength to NaN (#709) * do not set ContentLength to NaN * lint * docs: Add logo Signed-off-by: Richie Bendall <[email protected]> * chore: Update repository name from bitinn/node-fetch to node-fetch/node-fetch. Signed-off-by: Richie Bendall <[email protected]> * chore: Fix unit tests Signed-off-by: Richie Bendall <[email protected]> * chore(deps): Bump @pika/plugin-copy-assets from 0.7.1 to 0.8.1 (#713) Bumps [@pika/plugin-copy-assets](https://github.com/pikapkg/builders) from 0.7.1 to 0.8.1. - [Release notes](https://github.com/pikapkg/builders/releases) - [Commits](FredKSchott/pika-pack-builders@v0.7.1...v0.8.1) Signed-off-by: dependabot-preview[bot] <[email protected]> * chore(deps): Bump @pika/plugin-build-types from 0.7.1 to 0.8.1 (#710) Bumps [@pika/plugin-build-types](https://github.com/pikapkg/builders) from 0.7.1 to 0.8.1. - [Release notes](https://github.com/pikapkg/builders/releases) - [Commits](FredKSchott/pika-pack-builders@v0.7.1...v0.8.1) Signed-off-by: dependabot-preview[bot] <[email protected]> * Bump nyc from 14.1.1 to 15.0.0 (#714) Bumps [nyc](https://github.com/istanbuljs/nyc) from 14.1.1 to 15.0.0. - [Release notes](https://github.com/istanbuljs/nyc/releases) - [Changelog](https://github.com/istanbuljs/nyc/blob/master/CHANGELOG.md) - [Commits](istanbuljs/nyc@v14.1.1...v15.0.0) Signed-off-by: dependabot-preview[bot] <[email protected]> * chore: Update travis ci url Signed-off-by: Richie Bendall <[email protected]> * chore(deps): Bump mocha from 6.2.2 to 7.0.0 (#711) Bumps [mocha](https://github.com/mochajs/mocha) from 6.2.2 to 7.0.0. - [Release notes](https://github.com/mochajs/mocha/releases) - [Changelog](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md) - [Commits](mochajs/mocha@v6.2.2...v7.0.0) Signed-off-by: dependabot-preview[bot] <[email protected]> * feat: Allow excluding a user agent in a fetch request by setting… (#715) Signed-off-by: Richie Bendall <[email protected]> * Bump @pika/plugin-build-node from 0.7.1 to 0.8.1 (#717) Bumps [@pika/plugin-build-node](https://github.com/pikapkg/builders) from 0.7.1 to 0.8.1. - [Release notes](https://github.com/pikapkg/builders/releases) - [Commits](FredKSchott/pika-pack-builders@v0.7.1...v0.8.1) Signed-off-by: dependabot-preview[bot] <[email protected]> * Bump @pika/plugin-standard-pkg from 0.7.1 to 0.8.1 (#716) Bumps [@pika/plugin-standard-pkg](https://github.com/pikapkg/builders) from 0.7.1 to 0.8.1. - [Release notes](https://github.com/pikapkg/builders/releases) - [Commits](FredKSchott/pika-pack-builders@v0.7.1...v0.8.1) Signed-off-by: dependabot-preview[bot] <[email protected]> * Bump form-data from 2.5.1 to 3.0.0 (#712) Bumps [form-data](https://github.com/form-data/form-data) from 2.5.1 to 3.0.0. - [Release notes](https://github.com/form-data/form-data/releases) - [Commits](form-data/form-data@v2.5.1...v3.0.0) Signed-off-by: dependabot-preview[bot] <[email protected]> * fix: typo * update suggestion * feat: Added missing redirect function (#718) * added missing redirect function * chore: Add types Co-authored-by: Richie Bendall <[email protected]> * fix: Use req.setTimeout for timeout (#719) * chore: Update typings comment Signed-off-by: Richie Bendall <[email protected]> * chore: Update deps Signed-off-by: Richie Bendall <[email protected]> * docs: center badges & Open Collective button * docs: add missing comma * Remove current stable & LTS node version numbers from the comments I don't think we really want to update them * Bump xo from 0.25.4 to 0.26.1 (#730) Bumps [xo](https://github.com/xojs/xo) from 0.25.4 to 0.26.1. - [Release notes](https://github.com/xojs/xo/releases) - [Commits](xojs/xo@v0.25.4...v0.26.1) Signed-off-by: dependabot-preview[bot] <[email protected]> * Bump @pika/plugin-build-types from 0.8.3 to 0.9.2 (#729) Bumps [@pika/plugin-build-types](https://github.com/pikapkg/builders) from 0.8.3 to 0.9.2. - [Release notes](https://github.com/pikapkg/builders/releases) - [Commits](FredKSchott/pika-pack-builders@v0.8.3...v0.9.2) Signed-off-by: dependabot-preview[bot] <[email protected]> * Bump @pika/plugin-standard-pkg from 0.8.3 to 0.9.2 (#726) Bumps [@pika/plugin-standard-pkg](https://github.com/pikapkg/builders) from 0.8.3 to 0.9.2. - [Release notes](https://github.com/pikapkg/builders/releases) - [Commits](FredKSchott/pika-pack-builders@v0.8.3...v0.9.2) Signed-off-by: dependabot-preview[bot] <[email protected]> * docs: Update information about `req.body` type in v3.x release * Add information about removed body type to the v3 upgrade guide * add awesome badge * Show 2 ways of importing node-fetch (CommonJS & ES module) * update dependencies * lint * refactor: Replace `url.parse` with `new URL()` (#701) * chore: replace `url.parse` with `new URL()` * lint * handle relative URLs * Change error message * detect whether the url is absolute or not * update tests * drop relative url support * lint * fix tests * typo * Add information about dropped arbitrary URL support in v3.x upgrade guide * set xo linting rule (node/no-deprecated-api) to on * remove the `utf8` dependency * fix * refactor: split tests into several files, create the `utils` directory * Update package.json scripts & remove unnecessary xo linting rules * refactor: turn on some xo linting rules to improve code quality * fix tests * Remove invalid urls * fix merge conflict * update the upgrade guide * test if URLs are encoded as UTF-8 * update xo to 0.28.0 * chore: Build before publishing * v3.0.0-beta.1 * fix lint on test/main.js Co-authored-by: Richie Bendall <[email protected]> Co-authored-by: Antoni Kepinski <[email protected]> Co-authored-by: aeb-sia <[email protected]> Co-authored-by: Nazar Mokrynskyi <[email protected]> Co-authored-by: Steve Moser <[email protected]> Co-authored-by: Erick Calder <[email protected]> Co-authored-by: Yaacov Rydzinski <[email protected]> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> Co-authored-by: Jimmy Wärting <[email protected]>

SasanFarrokh · 2021-06-07T17:58:38Z

src/request.js

@@ -94,7 +115,7 @@ export default class Request {
 			signal = init.signal;
 		}

-		if (signal != null && !isAbortSignal(signal)) {
+		if (signal !== null && !isAbortSignal(signal)) {


This had made some issues, if signal is undefined, throws the error

LinusU · 2021-06-10T09:38:05Z

src/body.js

 	// Body is null or undefined
-	if (body == null) {
+	if (body === null) {


The comment is no longer accurate since this now only checks for null (=== null) and doesn't include undefined (== null)

This is present in a number of places...

chore: replace url.parse with new URL()

3b452b8

xxczaki added the help wanted label Dec 2, 2019

bitinn mentioned this pull request Jan 4, 2020

v3 Roadmap #668

Closed

35 tasks

Richienb changed the title ~~Replace url.parse with new URL()~~ refactor: Replace url.parse with new URL() Jan 5, 2020

xxczaki added 3 commits January 7, 2020 18:21

Merge branch '3.x' into whatwg-url

dd148fc

lint

4298eab

handle relative URLs

f414fb0

Richienb mentioned this pull request Jan 23, 2020

Confusion about whether utf8.encode(url) or not #722

Closed

xxczaki added 2 commits February 19, 2020 20:23

Change error message

eb9bed4

detect whether the url is absolute or not

4a795e7

xxczaki added 3 commits March 7, 2020 21:11

update tests

7c92e6e

drop relative url support

0a891fb

lint

1e747be

fix tests

dec4065

xxczaki added 4 commits March 8, 2020 23:10

refactor: split tests into several files, create the utils directory

deeb85b

Update package.json scripts & remove unnecessary xo linting rules

886b3d5

refactor: turn on some xo linting rules to improve code quality

147da1c

fix tests

42631e2

xxczaki added 5 commits March 8, 2020 23:30

Remove invalid urls

ac60e7a

Merge branch '3.x' into whatwg-url

54a04b7

Merge branch '3.x' into whatwg-url

3002b7f

fix merge conflict

0f07427

Merge branch 'whatwg-url' of https://github.com/node-fetch/node-fetch …

b102ffe

…into whatwg-url

bitinn requested changes Mar 9, 2020

View reviewed changes

update the upgrade guide

e1940b0

test if URLs are encoded as UTF-8

05bb4c8

xxczaki merged commit 0930d6d into 3.x Mar 13, 2020

xxczaki deleted the whatwg-url branch March 13, 2020 10:42

SasanFarrokh reviewed Jun 7, 2021

View reviewed changes

SasanFarrokh mentioned this pull request Jun 7, 2021

Throws error when passing signal as undefined #1182

Open

LinusU reviewed Jun 10, 2021

View reviewed changes

LinusU mentioned this pull request Sep 14, 2021

fix: properly encode url with unicode characters #1291

Merged

1 task

LinusU mentioned this pull request Sep 21, 2021

Problem with query params #1300

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Replace `url.parse` with `new URL()` #701

refactor: Replace `url.parse` with `new URL()` #701

xxczaki commented Dec 2, 2019

bitinn commented Dec 6, 2019 •

edited

Loading

Richienb commented Dec 13, 2019

bitinn commented Dec 15, 2019

Richienb commented Dec 15, 2019

bitinn commented Dec 15, 2019

Richienb commented Dec 15, 2019

bitinn commented Dec 15, 2019

xxczaki commented Jan 7, 2020 •

edited

Loading

Richienb commented Jan 8, 2020 •

edited

Loading

bitinn commented Jan 23, 2020 •

edited

Loading

xxczaki commented Feb 19, 2020 •

edited

Loading

bitinn commented Feb 20, 2020 via email

xxczaki commented Mar 7, 2020

bitinn commented Mar 8, 2020

jimmywarting commented Mar 8, 2020

bitinn commented Mar 8, 2020 •

edited

Loading

xxczaki commented Mar 8, 2020

bitinn left a comment

bitinn Mar 9, 2020

xxczaki Mar 9, 2020

xxczaki commented Mar 9, 2020 •

edited

Loading

jimmywarting commented Mar 9, 2020

xxczaki commented Mar 9, 2020

bitinn commented Mar 9, 2020

jimmywarting commented Mar 9, 2020 •

edited

Loading

xxczaki commented Mar 9, 2020

bitinn commented Mar 11, 2020

xxczaki commented Mar 12, 2020

bitinn commented Mar 13, 2020 via email

SasanFarrokh Jun 7, 2021

LinusU Jun 10, 2021

refactor: Replace url.parse with new URL() #701

refactor: Replace url.parse with new URL() #701

Conversation

xxczaki commented Dec 2, 2019

bitinn commented Dec 6, 2019 • edited Loading

Richienb commented Dec 13, 2019

bitinn commented Dec 15, 2019

Richienb commented Dec 15, 2019

bitinn commented Dec 15, 2019

Richienb commented Dec 15, 2019

bitinn commented Dec 15, 2019

xxczaki commented Jan 7, 2020 • edited Loading

Richienb commented Jan 8, 2020 • edited Loading

bitinn commented Jan 23, 2020 • edited Loading

xxczaki commented Feb 19, 2020 • edited Loading

bitinn commented Feb 20, 2020 via email

xxczaki commented Mar 7, 2020

bitinn commented Mar 8, 2020

jimmywarting commented Mar 8, 2020

bitinn commented Mar 8, 2020 • edited Loading

xxczaki commented Mar 8, 2020

bitinn left a comment

Choose a reason for hiding this comment

bitinn Mar 9, 2020

Choose a reason for hiding this comment

xxczaki Mar 9, 2020

Choose a reason for hiding this comment

xxczaki commented Mar 9, 2020 • edited Loading

jimmywarting commented Mar 9, 2020

xxczaki commented Mar 9, 2020

bitinn commented Mar 9, 2020

jimmywarting commented Mar 9, 2020 • edited Loading

xxczaki commented Mar 9, 2020

bitinn commented Mar 11, 2020

xxczaki commented Mar 12, 2020

bitinn commented Mar 13, 2020 via email

SasanFarrokh Jun 7, 2021

Choose a reason for hiding this comment

LinusU Jun 10, 2021

Choose a reason for hiding this comment

refactor: Replace `url.parse` with `new URL()` #701

refactor: Replace `url.parse` with `new URL()` #701

bitinn commented Dec 6, 2019 •

edited

Loading

xxczaki commented Jan 7, 2020 •

edited

Loading

Richienb commented Jan 8, 2020 •

edited

Loading

bitinn commented Jan 23, 2020 •

edited

Loading

xxczaki commented Feb 19, 2020 •

edited

Loading

bitinn commented Mar 8, 2020 •

edited

Loading

xxczaki commented Mar 9, 2020 •

edited

Loading

jimmywarting commented Mar 9, 2020 •

edited

Loading