Skip to content

Commit

Permalink
refactor: Replace url.parse with new URL() (#701)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
xxczaki committed Mar 13, 2020
1 parent 52db528 commit 0930d6d
Show file tree
Hide file tree
Showing 15 changed files with 971 additions and 907 deletions.
3 changes: 1 addition & 2 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

Changelog
=========

Expand All @@ -14,7 +13,7 @@ Changelog
- Enhance: data URI support.
- Enhance: drop existing blob implementation code and use fetch-blob as dependency instead.
- Enhance: modernise the code behind `FetchError` and `AbortError`.
- Enhance: replace deprecated `url.parse()` and `url.replace()` with the new WHATWG `new URL()`
- Enhance: replace deprecated `url.parse()` and `url.replace()` with the new WHATWG's `new URL()`
- Enhance: allow excluding a `user-agent` in a fetch request by setting it's header to null.
- Fix: `Response.statusText` no longer sets a default message derived from the HTTP status code.
- Fix: missing response stream error events.
Expand Down
10 changes: 8 additions & 2 deletions docs/v3-UPGRADE-GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

# Enhancements

## Data URI support
Expand All @@ -85,9 +89,11 @@ We now use the new Node.js [WHATWG-compliant URL API][whatwg-nodejs-url], so UTF

Since the v3.x required at least Node.js 10, we can utilise the new API.

## `AbortError` now uses a w3c defined message
## Creating Request/Response objects with relative URLs is no longer supported

To stay spec-compliant, we changed the `AbortError` message to `The operation was aborted.`.
We introduced Node.js `new URL()` API in 3.x, because 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 in 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.

## Bundled TypeScript types

Expand Down
309 changes: 149 additions & 160 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,161 +1,150 @@
{
"name": "node-fetch",
"version": "2.6.0",
"description": "A light-weight module that brings window.fetch to node.js",
"main": "dist/index.js",
"module": "dist/index.mjs",
"types": "types/index.d.ts",
"files": [
"src/**/*",
"dist/**/*",
"types/**/*.d.ts"
],
"engines": {
"node": ">=10.0.0"
},
"scripts": {
"build": "pika-pack --out dist/",
"prepare": "npm run build",
"test": "cross-env BABEL_ENV=test mocha --require @babel/register --throw-deprecation test/test.js",
"report": "cross-env BABEL_ENV=coverage nyc --reporter lcov --reporter text mocha -R spec test/test.js",
"coverage": "cross-env BABEL_ENV=coverage nyc --reporter json --reporter text mocha -R spec test/test.js && codecov -f coverage/coverage-final.json",
"lint": "xo"
},
"repository": {
"type": "git",
"url": "https://github.com/node-fetch/node-fetch.git"
},
"keywords": [
"fetch",
"http",
"promise"
],
"author": "David Frank",
"license": "MIT",
"bugs": {
"url": "https://github.com/node-fetch/node-fetch/issues"
},
"homepage": "https://github.com/node-fetch/node-fetch",
"funding": {
"type": "opencollective",
"url": "https://opencollective.com/node-fetch"
},
"devDependencies": {
"@babel/core": "^7.8.7",
"@babel/preset-env": "^7.8.7",
"@babel/register": "^7.8.6",
"@pika/pack": "^0.5.0",
"@pika/plugin-build-node": "^0.9.2",
"@pika/plugin-build-types": "^0.9.2",
"@pika/plugin-copy-assets": "^0.9.2",
"@pika/plugin-standard-pkg": "^0.9.2",
"abort-controller": "^3.0.0",
"abortcontroller-polyfill": "^1.4.0",
"chai": "^4.2.0",
"chai-as-promised": "^7.1.1",
"chai-iterator": "^3.0.2",
"chai-string": "^1.5.0",
"codecov": "^3.6.5",
"cross-env": "^7.0.2",
"form-data": "^3.0.0",
"mocha": "^7.1.0",
"nyc": "^15.0.0",
"parted": "^0.1.1",
"promise": "^8.1.0",
"resumer": "0.0.0",
"string-to-arraybuffer": "^1.0.2",
"xo": "^0.27.2"
},
"dependencies": {
"data-uri-to-buffer": "^3.0.0",
"fetch-blob": "^1.0.5",
"utf8": "^3.0.0"
},
"@pika/pack": {
"pipeline": [
[
"@pika/plugin-standard-pkg"
],
[
"@pika/plugin-build-node"
],
[
"@pika/plugin-build-types"
],
[
"@pika/plugin-copy-assets",
{
"files": [
"externals.d.ts"
]
}
]
]
},
"xo": {
"envs": [
"node",
"browser"
],
"rules": {
"valid-jsdoc": 0,
"no-multi-assign": 0,
"complexity": 0,
"unicorn/prefer-spread": 0,
"promise/prefer-await-to-then": 0,
"no-mixed-operators": 0,
"eqeqeq": 0,
"no-eq-null": 0,
"no-negated-condition": 0,
"prefer-named-capture-group": 0,
"unicorn/catch-error-name": 0,
"node/no-deprecated-api": 1
},
"ignores": [
"dist"
],
"overrides": [
{
"files": "test/**/*.js",
"envs": [
"node",
"mocha"
],
"rules": {
"max-nested-callbacks": 0,
"no-unused-expressions": 0,
"eslint-comments/no-unused-disable": 0,
"new-cap": 0,
"guard-for-in": 0,
"no-new": 0
}
},
{
"files": "example.js",
"rules": {
"import/no-extraneous-dependencies": 0
}
}
]
},
"babel": {
"presets": [
[
"@babel/preset-env",
{
"targets": {
"node": true
}
}
]
]
},
"nyc": {
"require": [
"@babel/register"
],
"sourceMap": false,
"instrument": false
},
"runkitExampleFilename": "example.js"
}
"name": "node-fetch",
"version": "2.6.0",
"description": "A light-weight module that brings window.fetch to node.js",
"main": "dist/index.js",
"module": "dist/index.mjs",
"types": "types/index.d.ts",
"files": [
"src/**/*",
"dist/**/*",
"types/**/*.d.ts"
],
"engines": {
"node": ">=10.0.0"
},
"scripts": {
"build": "pika-pack --out dist/",
"prepare": "npm run build",
"test": "cross-env BABEL_ENV=test mocha --require @babel/register --throw-deprecation test/*.js",
"report": "cross-env BABEL_ENV=coverage nyc --reporter lcov --reporter text mocha -R spec test/*.js",
"coverage": "cross-env BABEL_ENV=coverage nyc --reporter json --reporter text mocha -R spec test/*.js && codecov -f coverage/coverage-final.json",
"lint": "xo"
},
"repository": {
"type": "git",
"url": "https://github.com/node-fetch/node-fetch.git"
},
"keywords": [
"fetch",
"http",
"promise"
],
"author": "David Frank",
"license": "MIT",
"bugs": {
"url": "https://github.com/node-fetch/node-fetch/issues"
},
"homepage": "https://github.com/node-fetch/node-fetch",
"funding": {
"type": "opencollective",
"url": "https://opencollective.com/node-fetch"
},
"devDependencies": {
"@babel/core": "^7.8.4",
"@babel/preset-env": "^7.8.4",
"@babel/register": "^7.8.3",
"@pika/pack": "^0.5.0",
"@pika/plugin-build-node": "^0.8.1",
"@pika/plugin-build-types": "^0.9.2",
"@pika/plugin-copy-assets": "^0.8.1",
"@pika/plugin-standard-pkg": "^0.9.2",
"abort-controller": "^3.0.0",
"abortcontroller-polyfill": "^1.3.0",
"chai": "^4.2.0",
"chai-as-promised": "^7.1.1",
"chai-iterator": "^3.0.2",
"chai-string": "^1.5.0",
"codecov": "^3.6.4",
"cross-env": "^7.0.0",
"form-data": "^3.0.0",
"mocha": "^7.0.0",
"nyc": "^15.0.0",
"parted": "^0.1.1",
"promise": "^8.0.3",
"resumer": "0.0.0",
"string-to-arraybuffer": "^1.0.2",
"xo": "^0.26.1"
},
"dependencies": {
"data-uri-to-buffer": "^3.0.0",
"fetch-blob": "^1.0.5"
},
"@pika/pack": {
"pipeline": [
[
"@pika/plugin-standard-pkg"
],
[
"@pika/plugin-build-node"
],
[
"@pika/plugin-build-types"
],
[
"@pika/plugin-copy-assets",
{
"files": [
"externals.d.ts"
]
}
]
]
},
"xo": {
"envs": [
"node",
"browser"
],
"rules": {
"complexity": 0,
"promise/prefer-await-to-then": 0,
"no-mixed-operators": 0,
"no-negated-condition": 0
},
"ignores": [
"dist"
],
"overrides": [
{
"files": "test/**/*.js",
"envs": [
"node",
"mocha"
],
"rules": {
"max-nested-callbacks": 0,
"no-unused-expressions": 0,
"new-cap": 0,
"guard-for-in": 0
}
},
{
"files": "example.js",
"rules": {
"import/no-extraneous-dependencies": 0
}
}
]
},
"babel": {
"presets": [
[
"@babel/preset-env",
{
"targets": {
"node": true
}
}
]
]
},
"nyc": {
"require": [
"@babel/register"
],
"sourceMap": false,
"instrument": false
},
"runkitExampleFilename": "example.js"
}
8 changes: 4 additions & 4 deletions src/body.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export default function Body(body, {
size = 0,
timeout = 0
} = {}) {
if (body == null) {
if (body === null) {
// Body is undefined or null
body = null;
} else if (isURLSearchParams(body)) {
Expand Down Expand Up @@ -293,7 +293,7 @@ export function clone(instance, highWaterMark) {
*/
export function extractContentType(body) {
// Body is null or undefined
if (body == null) {
if (body === null) {
return null;
}

Expand Down Expand Up @@ -342,7 +342,7 @@ export function extractContentType(body) {
*/
export function getTotalBytes({body}) {
// Body is null or undefined
if (body == null) {
if (body === null) {
return 0;
}

Expand Down Expand Up @@ -373,7 +373,7 @@ export function getTotalBytes({body}) {
* @returns {void}
*/
export function writeToStream(dest, {body}) {
if (body == null) {
if (body === null) {
// Body is null
dest.end();
} else if (isBlob(body)) {
Expand Down
1 change: 1 addition & 0 deletions src/errors/fetch-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export default class FetchError extends Error {

// When err.type is `system`, err.erroredSysCall contains system error and err.code contains system error code
if (systemError) {
// eslint-disable-next-line no-multi-assign
this.code = this.errno = systemError.code;
this.erroredSysCall = systemError;
}
Expand Down

0 comments on commit 0930d6d

Please sign in to comment.