From c7fe6f0dc8f655cf7415c64daf7496d82a059ea7 Mon Sep 17 00:00:00 2001 From: Lubomir Date: Sat, 9 Oct 2021 19:12:13 +0000 Subject: [PATCH 1/6] Add a warning when using .data in RequestInit --- src/request.js | 7 +++++++ test/request.js | 16 ++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/request.js b/src/request.js index 8336150c3..6fc56c41e 100644 --- a/src/request.js +++ b/src/request.js @@ -28,6 +28,8 @@ const isRequest = object => { ); }; +let didReqDataWarn = false; + /** * Request class * @@ -56,6 +58,11 @@ export default class Request extends Body { let method = init.method || input.method || 'GET'; method = method.toUpperCase(); + if ('data' in init && !didReqDataWarn) { + console.warn('.data is not a valid RequestInit property, use .body instead'); + didReqDataWarn = true; + } + // eslint-disable-next-line no-eq-null, eqeqeq if (((init.body != null || isRequest(input)) && input.body !== null) && (method === 'GET' || method === 'HEAD')) { diff --git a/test/request.js b/test/request.js index 9d14fd137..3c8ddbf92 100644 --- a/test/request.js +++ b/test/request.js @@ -281,4 +281,20 @@ describe('Request', () => { expect(result).to.equal('a=1'); }); }); + + it('should warn once when using .data', () => { + const originalWarn = console.warn; + const collectedWarns = []; + console.warn = arg => collectedWarns.push(arg); + + new Request(base, { + data: '' + }); + new Request(base, { + data: '' + }); + expect(collectedWarns).to.deep.equal(['.data is not a valid RequestInit property, use .body instead']); + + console.warn = originalWarn; + }); }); From f32ccab8e9ec2878fee4c82fbf865b044dd19b62 Mon Sep 17 00:00:00 2001 From: Lubomir Date: Sat, 9 Oct 2021 19:20:08 +0000 Subject: [PATCH 2/6] Add a warning when using .data in Response --- src/response.js | 8 ++++++++ test/response.js | 14 ++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/response.js b/src/response.js index eaba9a9e1..64400bde7 100644 --- a/src/response.js +++ b/src/response.js @@ -10,6 +10,8 @@ import {isRedirect} from './utils/is-redirect.js'; const INTERNALS = Symbol('Response internals'); +let didResDataWarn = false; + /** * Response class * @@ -127,6 +129,12 @@ export default class Response extends Body { get [Symbol.toStringTag]() { return 'Response'; } + get data() { + if (!didResDataWarn) { + console.warn('.data is not a valid Response property, use .json(), .text(), .arrayBuffer(), or .body instead'); + didResDataWarn = true; + } + } } Object.defineProperties(Response.prototype, { diff --git a/test/response.js b/test/response.js index 6f020b45b..32ae68ef8 100644 --- a/test/response.js +++ b/test/response.js @@ -249,4 +249,18 @@ describe('Response', () => { expect(res.status).to.equal(0); expect(res.statusText).to.equal(''); }); + + it('should warn once when using .data', () => { + const originalWarn = console.warn; + const collectedWarns = []; + console.warn = arg => collectedWarns.push(arg); + + const response = new Response('a'); + response.data + response.data + + expect(collectedWarns).to.deep.equal(['.data is not a valid Response property, use .json(), .text(), .arrayBuffer(), or .body instead']); + + console.warn = originalWarn; + }); }); From 0c83bfd1d36416d465a21355af20e348d8d2e3ed Mon Sep 17 00:00:00 2001 From: Lubomir Date: Wed, 13 Oct 2021 20:44:32 +0300 Subject: [PATCH 3/6] Switch custom solution for utils.deprecate --- src/request.js | 10 ++++++---- src/response.js | 14 +++++--------- test/request.js | 22 +++++++++++----------- test/response.js | 21 ++++++++++----------- 4 files changed, 32 insertions(+), 35 deletions(-) diff --git a/src/request.js b/src/request.js index 6fc56c41e..d93980463 100644 --- a/src/request.js +++ b/src/request.js @@ -12,6 +12,7 @@ import Headers from './headers.js'; import Body, {clone, extractContentType, getTotalBytes} from './body.js'; import {isAbortSignal} from './utils/is.js'; import {getSearch} from './utils/get-search.js'; +import {deprecate} from 'util'; const INTERNALS = Symbol('Request internals'); @@ -28,7 +29,9 @@ const isRequest = object => { ); }; -let didReqDataWarn = false; +const doBadDataWarn = deprecate(() => {}, + '.data is not a valid RequestInit property, use .body instead', + 'https://github.com/node-fetch/node-fetch/issues/1000 (request)'); /** * Request class @@ -58,9 +61,8 @@ export default class Request extends Body { let method = init.method || input.method || 'GET'; method = method.toUpperCase(); - if ('data' in init && !didReqDataWarn) { - console.warn('.data is not a valid RequestInit property, use .body instead'); - didReqDataWarn = true; + if ('data' in init) { + doBadDataWarn(); } // eslint-disable-next-line no-eq-null, eqeqeq diff --git a/src/response.js b/src/response.js index 64400bde7..fcb88df8e 100644 --- a/src/response.js +++ b/src/response.js @@ -7,11 +7,10 @@ import Headers from './headers.js'; import Body, {clone, extractContentType} from './body.js'; import {isRedirect} from './utils/is-redirect.js'; +import {deprecate} from 'util'; const INTERNALS = Symbol('Response internals'); -let didResDataWarn = false; - /** * Response class * @@ -129,12 +128,6 @@ export default class Response extends Body { get [Symbol.toStringTag]() { return 'Response'; } - get data() { - if (!didResDataWarn) { - console.warn('.data is not a valid Response property, use .json(), .text(), .arrayBuffer(), or .body instead'); - didResDataWarn = true; - } - } } Object.defineProperties(Response.prototype, { @@ -145,5 +138,8 @@ Object.defineProperties(Response.prototype, { redirected: {enumerable: true}, statusText: {enumerable: true}, headers: {enumerable: true}, - clone: {enumerable: true} + clone: {enumerable: true}, + data: {get: deprecate(() => {}, + '.data is not a valid Response property, use .json(), .text(), .arrayBuffer(), or .body instead', + 'https://github.com/node-fetch/node-fetch/issues/1000 (response)')} }); diff --git a/test/request.js b/test/request.js index 3c8ddbf92..db2dee904 100644 --- a/test/request.js +++ b/test/request.js @@ -8,6 +8,7 @@ import Blob from 'fetch-blob'; import {Request} from '../src/index.js'; import TestServer from './utils/server.js'; +import { exit } from 'process'; const {expect} = chai; @@ -282,19 +283,18 @@ describe('Request', () => { }); }); - it('should warn once when using .data', () => { - const originalWarn = console.warn; - const collectedWarns = []; - console.warn = arg => collectedWarns.push(arg); + it('should warn once when using .data (request)', () => new Promise(resolve => { + const listenerFunc = ev => { + process.off('warning', listenerFunc); + expect(ev.message).to.equal('.data is not a valid RequestInit property, use .body instead'); + resolve(); + }; + process.on('warning', listenerFunc); + + // eslint-disable-next-line no-new new Request(base, { data: '' }); - new Request(base, { - data: '' - }); - expect(collectedWarns).to.deep.equal(['.data is not a valid RequestInit property, use .body instead']); - - console.warn = originalWarn; - }); + })); }); diff --git a/test/response.js b/test/response.js index 32ae68ef8..412fec17b 100644 --- a/test/response.js +++ b/test/response.js @@ -250,17 +250,16 @@ describe('Response', () => { expect(res.statusText).to.equal(''); }); - it('should warn once when using .data', () => { - const originalWarn = console.warn; - const collectedWarns = []; - console.warn = arg => collectedWarns.push(arg); + it('should warn once when using .data (response)', () => new Promise(resolve => { + const listenerFunc = ev => { + process.off('warning', listenerFunc); + expect(ev.message).to.equal('.data is not a valid Response property, use .json(), .text(), .arrayBuffer(), or .body instead'); + resolve(); + }; - const response = new Response('a'); - response.data - response.data - - expect(collectedWarns).to.deep.equal(['.data is not a valid Response property, use .json(), .text(), .arrayBuffer(), or .body instead']); + process.on('warning', listenerFunc); - console.warn = originalWarn; - }); + const response = new Response('a'); + response.data; + })); }); From 47c0703f75300aea94319ba469546072133f7210 Mon Sep 17 00:00:00 2001 From: Lubomir Date: Sun, 17 Oct 2021 16:51:25 +0300 Subject: [PATCH 4/6] Remove unused line in request tests --- test/request.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/request.js b/test/request.js index db2dee904..7d1e5fcb6 100644 --- a/test/request.js +++ b/test/request.js @@ -8,7 +8,6 @@ import Blob from 'fetch-blob'; import {Request} from '../src/index.js'; import TestServer from './utils/server.js'; -import { exit } from 'process'; const {expect} = chai; From 5f5ccd3beddd36239515ddc63d42c20dcb8eb58f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jimmy=20Wa=CC=88rting?= Date: Sun, 19 Dec 2021 13:04:24 +0100 Subject: [PATCH 5/6] moved error handler into the body class --- src/body.js | 5 ++++- src/request.js | 3 ++- src/response.js | 6 +----- test/referrer.js | 2 +- test/request.js | 9 +++------ test/response.js | 12 ++++-------- 6 files changed, 15 insertions(+), 22 deletions(-) diff --git a/src/body.js b/src/body.js index 64b880d48..f298458c7 100644 --- a/src/body.js +++ b/src/body.js @@ -177,7 +177,10 @@ Object.defineProperties(Body.prototype, { arrayBuffer: {enumerable: true}, blob: {enumerable: true}, json: {enumerable: true}, - text: {enumerable: true} + text: {enumerable: true}, + data: {get: deprecate(() => {}, + `data doesn't exist, use json(), text(), arrayBuffer(), or body instead`, + 'https://github.com/node-fetch/node-fetch/issues/1000 (response)')} }); /** diff --git a/src/request.js b/src/request.js index d058ee97e..76d7576b2 100644 --- a/src/request.js +++ b/src/request.js @@ -6,7 +6,8 @@ * All spec algorithm step numbers are based on https://fetch.spec.whatwg.org/commit-snapshots/ae716822cb3a61843226cd090eefc6589446c1d2/. */ -import {deprecate, format as formatUrl} from 'node:url'; +import {format as formatUrl} from 'node:url'; +import {deprecate} from 'node:util'; import Headers from './headers.js'; import Body, {clone, extractContentType, getTotalBytes} from './body.js'; import {isAbortSignal} from './utils/is.js'; diff --git a/src/response.js b/src/response.js index 65ed9fd93..63af26711 100644 --- a/src/response.js +++ b/src/response.js @@ -7,7 +7,6 @@ import Headers from './headers.js'; import Body, {clone, extractContentType} from './body.js'; import {isRedirect} from './utils/is-redirect.js'; -import {deprecate} from 'util'; const INTERNALS = Symbol('Response internals'); @@ -138,8 +137,5 @@ Object.defineProperties(Response.prototype, { redirected: {enumerable: true}, statusText: {enumerable: true}, headers: {enumerable: true}, - clone: {enumerable: true}, - data: {get: deprecate(() => {}, - '.data is not a valid Response property, use .json(), .text(), .arrayBuffer(), or .body instead', - 'https://github.com/node-fetch/node-fetch/issues/1000 (response)')} + clone: {enumerable: true} }); diff --git a/test/referrer.js b/test/referrer.js index 35e6b93c5..4410065ea 100644 --- a/test/referrer.js +++ b/test/referrer.js @@ -127,7 +127,7 @@ describe('Request constructor', () => { expect(() => { const req = new Request('http://example.com', {referrer: 'foobar'}); expect.fail(req); - }).to.throw(TypeError, 'Invalid URL: foobar'); + }).to.throw(TypeError, /Invalid URL/); }); }); diff --git a/test/request.js b/test/request.js index 221f8e880..0f02bf769 100644 --- a/test/request.js +++ b/test/request.js @@ -285,13 +285,10 @@ describe('Request', () => { }); it('should warn once when using .data (request)', () => new Promise(resolve => { - const listenerFunc = ev => { - process.off('warning', listenerFunc); - expect(ev.message).to.equal('.data is not a valid RequestInit property, use .body instead'); + process.once('warning', (evt) => { + expect(evt.message).to.equal('.data is not a valid RequestInit property, use .body instead'); resolve(); - }; - - process.on('warning', listenerFunc); + }); // eslint-disable-next-line no-new new Request(base, { diff --git a/test/response.js b/test/response.js index 85c87cfba..fa8d03e6b 100644 --- a/test/response.js +++ b/test/response.js @@ -250,15 +250,11 @@ describe('Response', () => { }); it('should warn once when using .data (response)', () => new Promise(resolve => { - const listenerFunc = ev => { - process.off('warning', listenerFunc); - expect(ev.message).to.equal('.data is not a valid Response property, use .json(), .text(), .arrayBuffer(), or .body instead'); + process.once('warning', (evt) => { + expect(evt.message).to.equal(`data doesn\'t exist, use json(), text(), arrayBuffer(), or body instead`); resolve(); - }; - - process.on('warning', listenerFunc); + }); - const response = new Response('a'); - response.data; + new Response('a').data; })); }); From 70c2b1c86c8b8ad3e7edf530d81ee62a0b9a1828 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jimmy=20Wa=CC=88rting?= Date: Sun, 19 Dec 2021 15:48:39 +0100 Subject: [PATCH 6/6] lint fix --- src/body.js | 2 +- test/request.js | 2 +- test/response.js | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/body.js b/src/body.js index f298458c7..2bcd9be6e 100644 --- a/src/body.js +++ b/src/body.js @@ -179,7 +179,7 @@ Object.defineProperties(Body.prototype, { json: {enumerable: true}, text: {enumerable: true}, data: {get: deprecate(() => {}, - `data doesn't exist, use json(), text(), arrayBuffer(), or body instead`, + 'data doesn\'t exist, use json(), text(), arrayBuffer(), or body instead', 'https://github.com/node-fetch/node-fetch/issues/1000 (response)')} }); diff --git a/test/request.js b/test/request.js index 0f02bf769..16251e7b6 100644 --- a/test/request.js +++ b/test/request.js @@ -285,7 +285,7 @@ describe('Request', () => { }); it('should warn once when using .data (request)', () => new Promise(resolve => { - process.once('warning', (evt) => { + process.once('warning', evt => { expect(evt.message).to.equal('.data is not a valid RequestInit property, use .body instead'); resolve(); }); diff --git a/test/response.js b/test/response.js index fa8d03e6b..0e3b7c492 100644 --- a/test/response.js +++ b/test/response.js @@ -250,8 +250,8 @@ describe('Response', () => { }); it('should warn once when using .data (response)', () => new Promise(resolve => { - process.once('warning', (evt) => { - expect(evt.message).to.equal(`data doesn\'t exist, use json(), text(), arrayBuffer(), or body instead`); + process.once('warning', evt => { + expect(evt.message).to.equal('data doesn\'t exist, use json(), text(), arrayBuffer(), or body instead'); resolve(); });