From e9db8695232fa502557e0e710c4a02a52d50bc01 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Mon, 23 Jan 2017 07:54:28 -0800 Subject: [PATCH] Remove FOLLOW_SPEC option; make it the default behavior (#225) * Remove !FOLLOW_SPEC mode * Update UPGRADE-GUIDE * Add CHANGELOG entry --- CHANGELOG.md | 1 + UPGRADE-GUIDE.md | 42 ++++++++++++++----------- src/headers.js | 56 ++++++---------------------------- src/index.js | 10 ------ test/test.js | 79 +++++++++--------------------------------------- 5 files changed, 48 insertions(+), 140 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e2c404..011fe65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ This is a major release. Check [our upgrade guide](https://github.com/bitinn/nod - Major: Node.js 0.10.x support is dropped - Major: rewrite in transpiled ES2015 - Major: internal methods are no longer exposed +- Major: remove `headers.getAll()`; make `get()` return all headers (per spec) - Major: throw error when a GET/HEAD Request is constructed with a non-null body (per spec) - Major: `response.text()` no longer attempts to detect encoding, instead always opting for UTF-8 (per spec); use `response.textConverted()` for the old behavior - Major: make `response.json()` throw error instead of returning an empty object on 204 no-content respose (per spec; reverts behavior set in v1.6.2) diff --git a/UPGRADE-GUIDE.md b/UPGRADE-GUIDE.md index 592b2ed..3ea1342 100644 --- a/UPGRADE-GUIDE.md +++ b/UPGRADE-GUIDE.md @@ -28,36 +28,42 @@ to file an issue and we will be happy to help you solve the problem. ## Headers The main goal we have for the `Headers` class in v2 is to make it completely -spec-compliant. However, due to changes in the Fetch Standard itself, total -spec compliance would mean incompatibility with all current major browser -implementations. - -Therefore, in v2, only a limited set of changes was applied to preserve -compatibility with browsers by default. See [#181] for more information on why -a feature is enabled or disabled. +spec-compliant. ```js ////////////////////////////////////////////////////////////////////////////// -// If you are using an object as the initializer, all values will be -// stringified. For arrays, the members will be joined with a comma. +// `get()` now returns **all** headers, joined by a comma, instead of only the +// first one. Its original behavior can be emulated using +// `get().split(',')[0]`. + const headers = new Headers({ 'Abc': 'string', 'Multi': [ 'header1', 'header2' ] }); // before after +headers.get('Abc') => headers.get('Abc') => + 'string' 'string' headers.get('Multi') => headers.get('Multi') => 'header1'; 'header1,header2'; -headers.getAll('Multi') => headers.getAll('Multi') => - [ 'header1', 'header2' ]; [ 'header1,header2' ]; + headers.get('Multi').split(',')[0] => + 'header1'; -// Instead, to preserve the older behavior, you can use the header pair array -// syntax. -const headers = new Headers([ - [ 'Abc', 'string' ], - [ 'Multi', 'header1' ], - [ 'Multi', 'header2' ] -]); + +////////////////////////////////////////////////////////////////////////////// +// `getAll()` is removed. Its behavior in v1 can be emulated with +// `get().split(',')`. + +const headers = new Headers({ + 'Abc': 'string', + 'Multi': [ 'header1', 'header2' ] +}); + +// before after +headers.getAll('Multi') => headers.getAll('Multi') => + [ 'header1', 'header2' ]; throws ReferenceError + headers.get('Multi').split(',') => + [ 'header1', 'header2' ]; ////////////////////////////////////////////////////////////////////////////// diff --git a/src/headers.js b/src/headers.js index 020ca70..39f839a 100644 --- a/src/headers.js +++ b/src/headers.js @@ -23,8 +23,7 @@ function sanitizeValue(value) { return value; } -export const MAP = Symbol('map'); -const FOLLOW_SPEC = Symbol('followSpec'); +const MAP = Symbol('map'); export default class Headers { /** * Headers class @@ -34,17 +33,8 @@ export default class Headers { */ constructor(headers) { this[MAP] = Object.create(null); - this[FOLLOW_SPEC] = Headers.FOLLOW_SPEC; - // Headers - if (headers instanceof Headers) { - let init = headers.raw(); - for (let name of Object.keys(init)) { - for (let value of init[name]) { - this.append(name, value); - } - } - } else if (typeof headers === 'object' && headers[Symbol.iterator]) { + if (typeof headers === 'object' && headers[Symbol.iterator]) { // array of tuples for (let el of headers) { if (typeof el !== 'object' || !el[Symbol.iterator]) { @@ -87,21 +77,7 @@ export default class Headers { return null; } - return this[FOLLOW_SPEC] ? list.join(',') : list[0]; - } - - /** - * Return all header values given name - * - * @param String name Header name - * @return Array - */ - getAll(name) { - if (!this.has(name)) { - return []; - } - - return this[MAP][sanitizeName(name)]; + return list.join(','); } /** @@ -217,24 +193,12 @@ Object.defineProperty(Headers.prototype, Symbol.toStringTag, { }); function getHeaderPairs(headers, kind) { - if (headers[FOLLOW_SPEC]) { - const keys = Object.keys(headers[MAP]).sort(); - return keys.map( - kind === 'key' ? - k => [k] : - k => [k, headers.get(k)] - ); - } - - const values = []; - - for (let name in headers[MAP]) { - for (let value of headers[MAP][name]) { - values.push([name, value]); - } - } - - return values; + const keys = Object.keys(headers[MAP]).sort(); + return keys.map( + kind === 'key' ? + k => [k] : + k => [k, headers.get(k)] + ); } const INTERNAL = Symbol('internal'); @@ -305,5 +269,3 @@ Object.defineProperty(HeadersIteratorPrototype, Symbol.toStringTag, { enumerable: false, configurable: true }); - -Headers.FOLLOW_SPEC = false; diff --git a/src/index.js b/src/index.js index ef668b8..ebd0dcb 100644 --- a/src/index.js +++ b/src/index.js @@ -32,7 +32,6 @@ export default function fetch(url, opts) { } Body.Promise = fetch.Promise; - Headers.FOLLOW_SPEC = fetch.FOLLOW_SPEC; // wrap http.request into fetch return new fetch.Promise((resolve, reject) => { @@ -199,15 +198,6 @@ fetch.isRedirect = code => code === 301 || code === 302 || code === 303 || code // expose Promise fetch.Promise = global.Promise; -/** - * Option to make newly constructed Headers objects conformant to the - * **latest** version of the Fetch Standard. Note, that most other - * implementations of fetch() have not yet been updated to the latest - * version, so enabling this option almost certainly breaks any isomorphic - * attempt. Also, changing this variable will only affect new Headers - * objects; existing objects are not affected. - */ -fetch.FOLLOW_SPEC = false; export { Headers, Request, diff --git a/test/test.js b/test/test.js index 129ed03..0d5a365 100644 --- a/test/test.js +++ b/test/test.js @@ -59,16 +59,7 @@ after(done => { local.stop(done); }); -(runner => { - runner(false); - runner(true); -})(defaultFollowSpec => { - -describe(`node-fetch with FOLLOW_SPEC = ${defaultFollowSpec}`, () => { - before(() => { - fetch.FOLLOW_SPEC = Headers.FOLLOW_SPEC = defaultFollowSpec; - }); - +describe('node-fetch', () => { it('should return a promise', function() { url = 'http://example.com/'; const p = fetch(url); @@ -1199,11 +1190,9 @@ describe(`node-fetch with FOLLOW_SPEC = ${defaultFollowSpec}`, () => { it('should allow get all responses of a header', function() { url = `${base}cookie`; return fetch(url).then(res => { - const expected = fetch.FOLLOW_SPEC ? 'a=1,b=1' : 'a=1'; + const expected = 'a=1,b=1'; expect(res.headers.get('set-cookie')).to.equal(expected); expect(res.headers.get('Set-Cookie')).to.equal(expected); - expect(res.headers.getAll('set-cookie')).to.deep.equal(['a=1', 'b=1']); - expect(res.headers.getAll('Set-Cookie')).to.deep.equal(['a=1', 'b=1']); }); }); @@ -1221,17 +1210,11 @@ describe(`node-fetch with FOLLOW_SPEC = ${defaultFollowSpec}`, () => { result.push([key, val]); }); - const expected = Headers.FOLLOW_SPEC ? [ + expect(result).to.deep.equal([ ["a", "1"] , ["b", "2,3"] , ["c", "4"] - ] : [ - ["b", "2"] - , ["b", "3"] - , ["c", "4"] - , ["a", "1"] - ]; - expect(result).to.deep.equal(expected); + ]); }); it('should allow iterating through all headers with for-of loop', function() { @@ -1247,15 +1230,10 @@ describe(`node-fetch with FOLLOW_SPEC = ${defaultFollowSpec}`, () => { for (let pair of headers) { result.push(pair); } - expect(result).to.deep.equal(Headers.FOLLOW_SPEC ? [ + expect(result).to.deep.equal([ ['a', '1'], ['b', '2,3'], ['c', '4'] - ] : [ - ['b', '2'], - ['b', '3'], - ['c', '4'], - ['a', '1'], ]); }); @@ -1268,15 +1246,10 @@ describe(`node-fetch with FOLLOW_SPEC = ${defaultFollowSpec}`, () => { headers.append('b', '3'); expect(headers.entries()).to.be.iterable - .and.to.deep.iterate.over(Headers.FOLLOW_SPEC ? [ + .and.to.deep.iterate.over([ ['a', '1'], ['b', '2,3'], ['c', '4'] - ] : [ - ['b', '2'], - ['b', '3'], - ['c', '4'], - ['a', '1'], ]); }); @@ -1289,7 +1262,7 @@ describe(`node-fetch with FOLLOW_SPEC = ${defaultFollowSpec}`, () => { headers.append('b', '3'); expect(headers.keys()).to.be.iterable - .and.to.iterate.over(Headers.FOLLOW_SPEC ? ['a', 'b', 'c'] : ['b', 'b', 'c', 'a']); + .and.to.iterate.over(['a', 'b', 'c']); }); it('should allow iterating through all headers with values()', function() { @@ -1301,27 +1274,7 @@ describe(`node-fetch with FOLLOW_SPEC = ${defaultFollowSpec}`, () => { headers.append('b', '3'); expect(headers.values()).to.be.iterable - .and.to.iterate.over(Headers.FOLLOW_SPEC ? ['1', '2,3', '4'] : ['2', '3', '4', '1']); - }); - - it('should only apply FOLLOW_SPEC when it is requested', function () { - Headers.FOLLOW_SPEC = true; - - const src = [ - ['b', '2'], - ['b', '3'] - ]; - - let headers = new Headers(src); - expect(headers.get('b')).to.equal('2,3'); - - Headers.FOLLOW_SPEC = false; - expect(headers.get('b')).to.equal('2,3'); - - headers = new Headers(src); - expect(headers.get('b')).to.equal('2'); - - Headers.FOLLOW_SPEC = defaultFollowSpec; + .and.to.iterate.over(['1', '2,3', '4']); }); it('should allow deleting header', function() { @@ -1329,7 +1282,6 @@ describe(`node-fetch with FOLLOW_SPEC = ${defaultFollowSpec}`, () => { return fetch(url).then(res => { res.headers.delete('set-cookie'); expect(res.headers.get('set-cookie')).to.be.null; - expect(res.headers.getAll('set-cookie')).to.be.empty; }); }); @@ -1341,7 +1293,6 @@ describe(`node-fetch with FOLLOW_SPEC = ${defaultFollowSpec}`, () => { expect(() => headers.append('Hé-y', 'ok')) .to.throw(TypeError); expect(() => headers.delete('Hé-y')) .to.throw(TypeError); expect(() => headers.get('Hé-y')) .to.throw(TypeError); - expect(() => headers.getAll('Hé-y')) .to.throw(TypeError); expect(() => headers.has('Hé-y')) .to.throw(TypeError); expect(() => headers.set('Hé-y', 'ok')) .to.throw(TypeError); @@ -1443,23 +1394,23 @@ describe(`node-fetch with FOLLOW_SPEC = ${defaultFollowSpec}`, () => { ['b', '2'], ['a', '3'] ]); - expect(headers.getAll('a')).to.deep.equal(['1', '3']); - expect(headers.getAll('b')).to.deep.equal(['2']); + expect(headers.get('a')).to.equal('1,3'); + expect(headers.get('b')).to.equal('2'); headers = new Headers([ new Set(['a', '1']), ['b', '2'], new Map([['a', null], ['3', null]]).keys() ]); - expect(headers.getAll('a')).to.deep.equal(['1', '3']); - expect(headers.getAll('b')).to.deep.equal(['2']); + expect(headers.get('a')).to.equal('1,3'); + expect(headers.get('b')).to.equal('2'); headers = new Headers(new Map([ ['a', '1'], ['b', '2'] ])); - expect(headers.getAll('a')).to.deep.equal(['1']); - expect(headers.getAll('b')).to.deep.equal(['2']); + expect(headers.get('a')).to.equal('1'); + expect(headers.get('b')).to.equal('2'); }); it('should throw a TypeError if non-tuple exists in a headers initializer', function() { @@ -1868,8 +1819,6 @@ describe(`node-fetch with FOLLOW_SPEC = ${defaultFollowSpec}`, () => { }); -}); - function streamToPromise(stream, dataHandler) { return new Promise((resolve, reject) => { stream.on('data', (...args) => {