From 5c6571fc60ceee67e4ba03b76afbf595134829cc Mon Sep 17 00:00:00 2001 From: Timothy Younger Date: Mon, 16 Oct 2017 15:04:09 -0700 Subject: [PATCH 1/8] Adds support for symbols as keys in Maps. --- __tests__/Map.ts | 23 +++++++++++++++++++++++ src/Seq.js | 4 +++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/__tests__/Map.ts b/__tests__/Map.ts index 50392b2c47..9b57a2df73 100644 --- a/__tests__/Map.ts +++ b/__tests__/Map.ts @@ -192,11 +192,14 @@ describe('Map', () => { }); it('can use weird keys', () => { + const symbol = Symbol('A'); const m: Map = Map() .set(NaN, 1) .set(Infinity, 2) + .set(symbol, 'A') .set(-Infinity, 3); + expect(m.get(symbol)).toBe('A'); expect(m.get(NaN)).toBe(1); expect(m.get(Infinity)).toBe(2); expect(m.get(-Infinity)).toBe(3); @@ -384,4 +387,24 @@ describe('Map', () => { expect(map.toString()).toEqual('Map { 2: 2 }'); }); + it('supports symbols as keys', () => { + const symbolA = Symbol('A'); + const symbolB = Symbol('B'); + const symbolC = Symbol('C'); + const m = Map({[symbolA]: 'A', [symbolB]: 'B', [symbolC]: 'C'}); + expect(m.size).toBe(3); + expect(m.get(symbolA)).toBe('A'); + expect(m.get(symbolB)).toBe('B'); + expect(m.get(symbolC)).toBe('C'); + }); + + it('symbol keys are unique', () => { + const symbolA = Symbol('FooBar'); + const symbolB = Symbol('FooBar'); + const m = Map({[symbolA]: 'FizBuz', [symbolB]: 'FooBar'}); + expect(m.size).toBe(2); + expect(m.get(symbolA)).toBe('FizBuz'); + expect(m.get(symbolB)).toBe('FooBar'); + }); + }); diff --git a/src/Seq.js b/src/Seq.js index e2229c5104..be417903e5 100644 --- a/src/Seq.js +++ b/src/Seq.js @@ -192,7 +192,9 @@ export class ArraySeq extends IndexedSeq { class ObjectSeq extends KeyedSeq { constructor(object) { - const keys = Object.keys(object); + const keys = Object.keys(object).concat( + Object.getOwnPropertySymbols(object) + ); this._object = object; this._keys = keys; this.size = keys.length; From 7b7b799db2d737647d0c118a140d2b8000ea73f1 Mon Sep 17 00:00:00 2001 From: Timothy Younger Date: Tue, 17 Oct 2017 09:02:05 -0700 Subject: [PATCH 2/8] checks for existence of `Object.getOwnPropertySymbols` before using it, for compat with IE and older browsers. --- src/Seq.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Seq.js b/src/Seq.js index be417903e5..50058e6e29 100644 --- a/src/Seq.js +++ b/src/Seq.js @@ -192,9 +192,11 @@ export class ArraySeq extends IndexedSeq { class ObjectSeq extends KeyedSeq { constructor(object) { - const keys = Object.keys(object).concat( - Object.getOwnPropertySymbols(object) - ); + let symbols = []; + if (typeof Object.getOwnPropertySymbols === 'function') { + symbols = Object.getOwnPropertySymbols(object); + } + const keys = Object.keys(object).concat(symbols); this._object = object; this._keys = keys; this.size = keys.length; From 9151ed6aa2d513449b8e87a160c3aff934cdced3 Mon Sep 17 00:00:00 2001 From: Timothy Younger Date: Tue, 17 Oct 2017 12:04:58 -0700 Subject: [PATCH 3/8] avoid key concatenation in ObjectSeq when Symbols are not available. --- src/Seq.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Seq.js b/src/Seq.js index 50058e6e29..43cf0648a0 100644 --- a/src/Seq.js +++ b/src/Seq.js @@ -192,11 +192,12 @@ export class ArraySeq extends IndexedSeq { class ObjectSeq extends KeyedSeq { constructor(object) { - let symbols = []; + let keys = []; if (typeof Object.getOwnPropertySymbols === 'function') { - symbols = Object.getOwnPropertySymbols(object); + keys = Object.keys(object).concat(Object.getOwnPropertySymbols(object)); + } else { + keys = Object.keys(object); } - const keys = Object.keys(object).concat(symbols); this._object = object; this._keys = keys; this.size = keys.length; From cf479f7a8cb98506eef570d321c9547d6252e8e3 Mon Sep 17 00:00:00 2001 From: Timothy Younger Date: Tue, 17 Oct 2017 15:20:27 -0700 Subject: [PATCH 4/8] update shallowCopy to handle Symbol keys in objects. --- __tests__/merge.ts | 52 ++++++++++++++++++++++++++++++++++++++++ src/utils/shallowCopy.js | 5 ++++ 2 files changed, 57 insertions(+) diff --git a/__tests__/merge.ts b/__tests__/merge.ts index 7efe15be9b..e4c586e5e0 100644 --- a/__tests__/merge.ts +++ b/__tests__/merge.ts @@ -200,4 +200,56 @@ describe('merge', () => { expect(merge(a, [], [])).toBe(a); }); + it('merge JS Objects with Symbol keys', () => { + const c = Symbol('c'); + const d = Symbol('d'); + const e = Symbol('e'); + expect( + merge({[c]: 1, [d]: 2 }, {[c]: 10, [e]: 20}), + ).toEqual( + {[c]: 10, [d]: 2, [e]: 20}, + ); + }); + + it('mergeDeep JS Objects with Symbol keys', () => { + const a = Symbol('a'); + const b = Symbol('b'); + const c = Symbol('c'); + const d = Symbol('d'); + const e = Symbol('e'); + const f = Symbol('f'); + const g = Symbol('g'); + expect( + mergeDeep({[a]: {[b]: {[c]: 1, [d]: 2}}}, {[a]: {[b]: {[c]: 10, [e]: 20}, [f]: 30}, [g]: 40}), + ).toEqual( + {[a]: {[b]: {[c]: 10, [d]: 2, [e]: 20}, [f]: 30}, [g]: 40}, + ); + }); + + it('merge Maps with Symbol keys', () => { + const c = Symbol('c'); + const d = Symbol('d'); + const e = Symbol('e'); + expect( + merge(Map({[c]: 1, [d]: 2 }), Map({[c]: 10, [e]: 20})), + ).toEqual( + Map({[c]: 10, [d]: 2, [e]: 20}), + ); + }); + + it('mergeDeep Maps with Symbol keys', () => { + const a = Symbol('a'); + const b = Symbol('b'); + const c = Symbol('c'); + const d = Symbol('d'); + const e = Symbol('e'); + const f = Symbol('f'); + const g = Symbol('g'); + const m1 = Map({[a]: {[b]: {[c]: 1, [d]: 2}}}); + const m2 = Map({[a]: {[b]: {[c]: 10, [e]: 20}, [f]: 30}, [g]: 40}); + const actual = m1.mergeDeep(m2); + const expected = Map({[a]: {[b]: {[c]: 10, [d]: 2, [e]: 20}, [f]: 30}, [g]: 40}); + expect(actual).toEqual(expected); + }); + }); diff --git a/src/utils/shallowCopy.js b/src/utils/shallowCopy.js index 7cc6133f19..be946c871c 100644 --- a/src/utils/shallowCopy.js +++ b/src/utils/shallowCopy.js @@ -18,5 +18,10 @@ export default function shallowCopy(from) { to[key] = from[key]; } } + if (typeof Object.getOwnPropertySymbols === 'function') { + Object.getOwnPropertySymbols(from).forEach(key => { + to[key] = from[key]; + }); + } return to; } From c0629bc5a41c604354f791671a9c2b02aac282c0 Mon Sep 17 00:00:00 2001 From: Timothy Younger Date: Tue, 17 Oct 2017 16:14:41 -0700 Subject: [PATCH 5/8] consolidate ObjectSeq key initializer and remove unnecessary else condition. --- src/Seq.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Seq.js b/src/Seq.js index 43cf0648a0..a3c6cf8829 100644 --- a/src/Seq.js +++ b/src/Seq.js @@ -192,11 +192,9 @@ export class ArraySeq extends IndexedSeq { class ObjectSeq extends KeyedSeq { constructor(object) { - let keys = []; + let keys = Object.keys(object); if (typeof Object.getOwnPropertySymbols === 'function') { - keys = Object.keys(object).concat(Object.getOwnPropertySymbols(object)); - } else { - keys = Object.keys(object); + keys = keys.concat(Object.getOwnPropertySymbols(object)); } this._object = object; this._keys = keys; From 7dce2e41b3178929eabfbb3a740d5dbd90fb1319 Mon Sep 17 00:00:00 2001 From: Timothy Younger Date: Wed, 18 Oct 2017 09:12:24 -0700 Subject: [PATCH 6/8] Skip concat in Map instantiation and loop in shallowCopy when objects do not have any Symbols. --- src/Seq.js | 5 ++++- src/utils/shallowCopy.js | 9 ++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Seq.js b/src/Seq.js index a3c6cf8829..05fa52b938 100644 --- a/src/Seq.js +++ b/src/Seq.js @@ -194,7 +194,10 @@ class ObjectSeq extends KeyedSeq { constructor(object) { let keys = Object.keys(object); if (typeof Object.getOwnPropertySymbols === 'function') { - keys = keys.concat(Object.getOwnPropertySymbols(object)); + const symbols = Object.getOwnPropertySymbols(object); + if (symbols.length > 0) { + keys = keys.concat(symbols); + } } this._object = object; this._keys = keys; diff --git a/src/utils/shallowCopy.js b/src/utils/shallowCopy.js index be946c871c..1894e12231 100644 --- a/src/utils/shallowCopy.js +++ b/src/utils/shallowCopy.js @@ -19,9 +19,12 @@ export default function shallowCopy(from) { } } if (typeof Object.getOwnPropertySymbols === 'function') { - Object.getOwnPropertySymbols(from).forEach(key => { - to[key] = from[key]; - }); + const symbols = Object.getOwnPropertySymbols(from); + if (symbols.length > 0) { + symbols.forEach(key => { + to[key] = from[key]; + }); + } } return to; } From 6f5a47f7fee0077599e3da799bbdc45d64880bca Mon Sep 17 00:00:00 2001 From: Timothy Younger Date: Wed, 25 Oct 2017 21:43:20 -0700 Subject: [PATCH 7/8] removes Symbol-handling logic from Seq and shallowCopy due to performance issues. adds named constructor to Map which recursively converts object literal KV pairs into [K, V] form, in which Map correctly handles Symbol keys. --- __tests__/Map.ts | 56 ++++++++++++++++++++++++++++++---------- __tests__/merge.ts | 54 -------------------------------------- src/Map.js | 8 ++++++ src/Seq.js | 8 +----- src/utils/shallowCopy.js | 8 ------ 5 files changed, 51 insertions(+), 83 deletions(-) diff --git a/__tests__/Map.ts b/__tests__/Map.ts index 6481df1fe7..10f9901fc5 100644 --- a/__tests__/Map.ts +++ b/__tests__/Map.ts @@ -468,23 +468,51 @@ describe('Map', () => { ); }); - it('supports symbols as keys', () => { - const symbolA = Symbol('A'); - const symbolB = Symbol('B'); - const symbolC = Symbol('C'); - const m = Map({ [symbolA]: 'A', [symbolB]: 'B', [symbolC]: 'C' }); + it('supports Symbol keys from object literal', () => { + const a = Symbol('A'); + const b = Symbol('B'); + const c = Symbol('C'); + const m = Map.ownEntries({ [a]: 'A', [b]: 'B', [c]: 'C' }); expect(m.size).toBe(3); - expect(m.get(symbolA)).toBe('A'); - expect(m.get(symbolB)).toBe('B'); - expect(m.get(symbolC)).toBe('C'); + expect(m.get(a)).toBe('A'); + expect(m.get(b)).toBe('B'); + expect(m.get(c)).toBe('C'); }); - it('symbol keys are unique', () => { - const symbolA = Symbol('FooBar'); - const symbolB = Symbol('FooBar'); - const m = Map({ [symbolA]: 'FizBuz', [symbolB]: 'FooBar' }); + it('Symbol keys from object literal are unique', () => { + const a = Symbol('FooBar'); + const b = Symbol('FooBar'); + const m = Map.ownEntries({ [a]: 'FizBuz', [b]: 'FooBar' }); expect(m.size).toBe(2); - expect(m.get(symbolA)).toBe('FizBuz'); - expect(m.get(symbolB)).toBe('FooBar'); + expect(m.get(a)).toBe('FizBuz'); + expect(m.get(b)).toBe('FooBar'); + }); + + it('mergeDeep Map from object literal', () => { + const a = Symbol('a'); + const b = Symbol('b'); + const c = Symbol('c'); + const d = Symbol('d'); + const e = Symbol('e'); + const f = Symbol('f'); + const g = Symbol('g'); + const m1 = Map.ownEntries({ [a]: { [b]: { [c]: 1, [d]: 2 } } }); + const m2 = Map.ownEntries({ + [a]: { [b]: { [c]: 10, [e]: 20 }, [f]: 30 }, + [g]: 40, + }); + const expected = Map.ownEntries({ + [a]: { [b]: { [c]: 10, [d]: 2, [e]: 20 }, [f]: 30 }, + [g]: 40, + }); + const actual = m1.mergeDeep(m2); + expect(actual).toEqual(expected); + + const actualJs = actual.toJS(); + expect(actualJs[a][b][c]).toEqual(10); + expect(actualJs[a][b][d]).toEqual(2); + expect(actualJs[a][b][e]).toEqual(20); + expect(actualJs[a][f]).toEqual(30); + expect(actualJs[g]).toEqual(40); }); }); diff --git a/__tests__/merge.ts b/__tests__/merge.ts index f8a6cd1462..5f6247b2e8 100644 --- a/__tests__/merge.ts +++ b/__tests__/merge.ts @@ -229,58 +229,4 @@ describe('merge', () => { Map([[a, Map([[b, Map([[c, 10], [d, 2], [e, 20], [f, 30], [g, 40]])]])]]) ); }); - - it('merge JS Objects with Symbol keys', () => { - const c = Symbol('c'); - const d = Symbol('d'); - const e = Symbol('e'); - expect(merge({ [c]: 1, [d]: 2 }, { [c]: 10, [e]: 20 })).toEqual({ - [c]: 10, - [d]: 2, - [e]: 20, - }); - }); - - it('mergeDeep JS Objects with Symbol keys', () => { - const a = Symbol('a'); - const b = Symbol('b'); - const c = Symbol('c'); - const d = Symbol('d'); - const e = Symbol('e'); - const f = Symbol('f'); - const g = Symbol('g'); - expect( - mergeDeep( - { [a]: { [b]: { [c]: 1, [d]: 2 } } }, - { [a]: { [b]: { [c]: 10, [e]: 20 }, [f]: 30 }, [g]: 40 } - ) - ).toEqual({ [a]: { [b]: { [c]: 10, [d]: 2, [e]: 20 }, [f]: 30 }, [g]: 40 }); - }); - - it('merge Maps with Symbol keys', () => { - const c = Symbol('c'); - const d = Symbol('d'); - const e = Symbol('e'); - expect(merge(Map({ [c]: 1, [d]: 2 }), Map({ [c]: 10, [e]: 20 }))).toEqual( - Map({ [c]: 10, [d]: 2, [e]: 20 }) - ); - }); - - it('mergeDeep Maps with Symbol keys', () => { - const a = Symbol('a'); - const b = Symbol('b'); - const c = Symbol('c'); - const d = Symbol('d'); - const e = Symbol('e'); - const f = Symbol('f'); - const g = Symbol('g'); - const m1 = Map({ [a]: { [b]: { [c]: 1, [d]: 2 } } }); - const m2 = Map({ [a]: { [b]: { [c]: 10, [e]: 20 }, [f]: 30 }, [g]: 40 }); - const actual = m1.mergeDeep(m2); - const expected = Map({ - [a]: { [b]: { [c]: 10, [d]: 2, [e]: 20 }, [f]: 30 }, - [g]: 40, - }); - expect(actual).toEqual(expected); - }); }); diff --git a/src/Map.js b/src/Map.js index 6a1e9f3350..8e26768917 100644 --- a/src/Map.js +++ b/src/Map.js @@ -66,6 +66,14 @@ export class Map extends KeyedCollection { }); } + static ownEntries(obj) { + if (typeof obj !== 'object') { + return obj; + } + + return Map(Reflect.ownKeys(obj).map(k => [k, Map.ownEntries(obj[k])])); + } + toString() { return this.__toString('Map {', '}'); } diff --git a/src/Seq.js b/src/Seq.js index 16e142631b..425294c6eb 100644 --- a/src/Seq.js +++ b/src/Seq.js @@ -192,13 +192,7 @@ export class ArraySeq extends IndexedSeq { class ObjectSeq extends KeyedSeq { constructor(object) { - let keys = Object.keys(object); - if (typeof Object.getOwnPropertySymbols === 'function') { - const symbols = Object.getOwnPropertySymbols(object); - if (symbols.length > 0) { - keys = keys.concat(symbols); - } - } + const keys = Object.keys(object); this._object = object; this._keys = keys; this.size = keys.length; diff --git a/src/utils/shallowCopy.js b/src/utils/shallowCopy.js index 1894e12231..7cc6133f19 100644 --- a/src/utils/shallowCopy.js +++ b/src/utils/shallowCopy.js @@ -18,13 +18,5 @@ export default function shallowCopy(from) { to[key] = from[key]; } } - if (typeof Object.getOwnPropertySymbols === 'function') { - const symbols = Object.getOwnPropertySymbols(from); - if (symbols.length > 0) { - symbols.forEach(key => { - to[key] = from[key]; - }); - } - } return to; } From c6dad009360621d5905749b67c28b01289bf58b1 Mon Sep 17 00:00:00 2001 From: Timothy Younger Date: Thu, 17 May 2018 21:51:25 -0700 Subject: [PATCH 8/8] Renames Map.ownEntries to Map.fromOwnEntries, adds explicit test case covering fromOwnEntries recursion, removes ambiguous return type, adds typings, and adds documentation with runkit examples. --- __tests__/Map.ts | 38 +++++++++++++++++++++++---- src/Map.js | 15 ++++++----- type-definitions/Immutable.d.ts | 42 ++++++++++++++++++++++++++++++ type-definitions/immutable.js.flow | 1 + 4 files changed, 85 insertions(+), 11 deletions(-) diff --git a/__tests__/Map.ts b/__tests__/Map.ts index cd263b9508..f6436f9cbc 100644 --- a/__tests__/Map.ts +++ b/__tests__/Map.ts @@ -478,7 +478,7 @@ describe('Map', () => { const a = Symbol('A'); const b = Symbol('B'); const c = Symbol('C'); - const m = Map.ownEntries({ [a]: 'A', [b]: 'B', [c]: 'C' }); + const m = Map.fromOwnEntries({ [a]: 'A', [b]: 'B', [c]: 'C' }); expect(m.size).toBe(3); expect(m.get(a)).toBe('A'); expect(m.get(b)).toBe('B'); @@ -488,12 +488,40 @@ describe('Map', () => { it('Symbol keys from object literal are unique', () => { const a = Symbol('FooBar'); const b = Symbol('FooBar'); - const m = Map.ownEntries({ [a]: 'FizBuz', [b]: 'FooBar' }); + const m = Map.fromOwnEntries({ [a]: 'FizBuz', [b]: 'FooBar' }); expect(m.size).toBe(2); expect(m.get(a)).toBe('FizBuz'); expect(m.get(b)).toBe('FooBar'); }); + it('creates nested Map from object with nested Symbol keys', () => { + const a = Symbol('a'); + const b = Symbol('b'); + const c = Symbol('c'); + const e = Symbol('e'); + const f = Symbol('f'); + const g = Symbol('g'); + const map = Map.fromOwnEntries({ + [a]: { [b]: { [c]: 10, [e]: 20 }, [f]: 30 }, + [g]: 40, + }); + + expect( + map + .get(a) + .get(b) + .get(c) + ).toEqual(10); + expect( + map + .get(a) + .get(b) + .get(e) + ).toEqual(20); + expect(map.get(a).get(f)).toEqual(30); + expect(map.get(g)).toEqual(40); + }); + it('mergeDeep Map from object literal', () => { const a = Symbol('a'); const b = Symbol('b'); @@ -502,12 +530,12 @@ describe('Map', () => { const e = Symbol('e'); const f = Symbol('f'); const g = Symbol('g'); - const m1 = Map.ownEntries({ [a]: { [b]: { [c]: 1, [d]: 2 } } }); - const m2 = Map.ownEntries({ + const m1 = Map.fromOwnEntries({ [a]: { [b]: { [c]: 1, [d]: 2 } } }); + const m2 = Map.fromOwnEntries({ [a]: { [b]: { [c]: 10, [e]: 20 }, [f]: 30 }, [g]: 40, }); - const expected = Map.ownEntries({ + const expected = Map.fromOwnEntries({ [a]: { [b]: { [c]: 10, [d]: 2, [e]: 20 }, [f]: 30 }, [g]: 40, }); diff --git a/src/Map.js b/src/Map.js index 924fd1a8d8..e7327895f6 100644 --- a/src/Map.js +++ b/src/Map.js @@ -66,12 +66,15 @@ export class Map extends KeyedCollection { }); } - static ownEntries(obj) { - if (typeof obj !== 'object') { - return obj; - } - - return Map(Reflect.ownKeys(obj).map(k => [k, Map.ownEntries(obj[k])])); + static fromOwnEntries(source) { + return Map( + Reflect.ownKeys(source).map(key => [ + key, + typeof source[key] === 'object' + ? Map.fromOwnEntries(source[key]) + : source[key], + ]) + ); } toString() { diff --git a/type-definitions/Immutable.d.ts b/type-definitions/Immutable.d.ts index a330bf4336..64c0c8d2fa 100644 --- a/type-definitions/Immutable.d.ts +++ b/type-definitions/Immutable.d.ts @@ -717,6 +717,48 @@ declare module Immutable { * @deprecated Use Map([ [ 'k', 'v' ] ]) or Map({ k: 'v' }) */ function of(...keyValues: Array): Map; + + /** + * Creates a new Map from an object. + * + * + * ```js + * const { Map } = require('immutable') + * Map.fromOwnEntries({ + * foo: 'bar', + * fiz: 'buz' + * }) + * // Map { "foo": "bar", "fiz": "buz" } + * ``` + * + * Nested objects are recursively converted into instances of Map. + * + * + * ```js + * const { Map } = require('immutable') + * Map.fromOwnEntries({ + * foo: 'bar', + * fiz: { + * buz: 'qux' + * } + * }) + * // Map { "foo": "bar", "fiz": Map { "buz": "qux" } } + * ``` + * + * Symbol keys are supported. + * + * + * ```js + * const { Map } = require('immutable') + * const fiz = Symbol('fiz'); + * Map.fromOwnEntries({ + * foo: 'bar', + * [fiz]: 'buz' + * }) + * // Map { "foo": "bar", "Symbol(fiz)": "buz" } + * ``` + */ + function fromOwnEntries(obj: any): Map; } /** diff --git a/type-definitions/immutable.js.flow b/type-definitions/immutable.js.flow index 9904310f83..d24a8441e1 100644 --- a/type-definitions/immutable.js.flow +++ b/type-definitions/immutable.js.flow @@ -868,6 +868,7 @@ declare class Map extends KeyedCollection mixins UpdatableInCollect size: number; + fromOwnEntries(obj: mixed): Map; set(key: K_, value: V_): Map; delete(key: K): this; remove(key: K): this;