From 74b5b1a943bd9fe307b12ec36f4ce71ab2c9998a Mon Sep 17 00:00:00 2001 From: Ben Lesh Date: Thu, 2 Nov 2017 19:56:25 -0700 Subject: [PATCH] refactor(groupBy): Remove Map polyfill - Removes Map polyfill - Removes FastMap impl - Tests in latest Chrome and Node show no discernable difference in performance between Map and Object has a hashtable for our use case. - Adds an error that is thrown immediately at runtime if Map does not exist. BREAKING CHANGE: Older runtimes will require Map to be polyfilled to use `groupBy` --- spec/util/FastMap-spec.ts | 81 ----------------------------------- spec/util/MapPolyfill-spec.ts | 78 --------------------------------- src/operators/groupBy.ts | 9 ++-- src/util/FastMap.ts | 30 ------------- src/util/Map.ts | 4 -- src/util/MapPolyfill.ts | 43 ------------------- 6 files changed, 6 insertions(+), 239 deletions(-) delete mode 100644 spec/util/FastMap-spec.ts delete mode 100644 spec/util/MapPolyfill-spec.ts delete mode 100644 src/util/FastMap.ts delete mode 100644 src/util/Map.ts delete mode 100644 src/util/MapPolyfill.ts diff --git a/spec/util/FastMap-spec.ts b/spec/util/FastMap-spec.ts deleted file mode 100644 index a9b8c520c6..0000000000 --- a/spec/util/FastMap-spec.ts +++ /dev/null @@ -1,81 +0,0 @@ -import { expect } from 'chai'; -import { FastMap } from '../../src/util/FastMap'; - -/** @test {FastMap} */ -describe('FastMap', () => { - it('should exist', () => { - expect(FastMap).to.be.a('function'); - }); - - it('should accept string as keys', () => { - const map = new FastMap(); - const key1 = 'keyOne'; - const key2 = 'keyTwo'; - - map.set(key1, 'yo'); - map.set(key2, 'what up'); - - expect(map.get(key1)).to.equal('yo'); - expect(map.get(key2)).to.equal('what up'); - }); - - it('should allow setting keys twice', () => { - const map = new FastMap(); - const key1 = 'keyOne'; - - map.set(key1, 'sing'); - map.set(key1, 'yodel'); - - expect(map.get(key1)).to.equal('yodel'); - }); - - it('should have a delete method that removes keys', () => { - const map = new FastMap(); - const key1 = 'keyOne'; - - map.set(key1, 'sing'); - - expect(map.delete(key1)).to.be.true; - expect(map.get(key1)).to.be.a('null'); - }); - - it('should clear all', () => { - const map = new FastMap(); - const key1 = 'keyOne'; - const key2 = 'keyTwo'; - - map.set(key1, 'yo'); - map.set(key2, 'what up'); - - map.clear(); - - expect(map.get(key1)).to.be.a('undefined'); - expect(map.get(key2)).to.be.a('undefined'); - }); - - describe('prototype.forEach', () => { - it('should exist', () => { - const map = new FastMap(); - expect(map.forEach).to.be.a('function'); - }); - - it('should iterate over keys and values', () => { - const expectedKeys = ['a', 'b', 'c']; - const expectedValues = [1, 2, 3]; - const map = new FastMap(); - map.set('a', 1); - map.set('b', 2); - map.set('c', 3); - const thisArg = {}; - - map.forEach(function (value, key) { - expect(this).to.equal(thisArg); - expect(value).to.equal(expectedValues.shift()); - expect(key).to.equal(expectedKeys.shift()); - }, thisArg); - - expect(expectedValues.length).to.equal(0); - expect(expectedKeys.length).to.equal(0); - }); - }); -}); diff --git a/spec/util/MapPolyfill-spec.ts b/spec/util/MapPolyfill-spec.ts deleted file mode 100644 index 2fcc33fffd..0000000000 --- a/spec/util/MapPolyfill-spec.ts +++ /dev/null @@ -1,78 +0,0 @@ -import { expect } from 'chai'; -import { MapPolyfill } from '../../src/util/MapPolyfill'; - -/** @test {MapPolyfill} */ -describe('MapPolyfill', () => { - it('should exist', () => { - expect(MapPolyfill).to.be.a('function'); - }); - - it('should act like a hashtable that accepts objects as keys', () => { - const map = new MapPolyfill(); - const key1 = {}; - const key2 = {}; - - map.set('test', 'hi'); - map.set(key1, 'yo'); - map.set(key2, 'what up'); - - expect(map.get('test')).to.equal('hi'); - expect(map.get(key1)).to.equal('yo'); - expect(map.get(key2)).to.equal('what up'); - expect(map.size).to.equal(3); - }); - - it('should allow setting keys twice', () => { - const map = new MapPolyfill(); - const key1 = {}; - - map.set(key1, 'sing'); - map.set(key1, 'yodel'); - - expect(map.get(key1)).to.equal('yodel'); - expect(map.size).to.equal(1); - }); - - it('should have a delete method that removes keys', () => { - const map = new MapPolyfill(); - const key1 = {}; - - map.set(key1, 'sing'); - expect(map.size).to.equal(1); - - map.delete(key1); - - expect(map.size).to.equal(0); - expect(map.get(key1)).to.be.a('undefined'); - }); - - describe('prototype.forEach', () => { - it('should exist', () => { - const map = new MapPolyfill(); - expect(map.forEach).to.be.a('function'); - }); - - it('should iterate over keys and values', () => { - const expectedKeys = ['a', 'b', 'c']; - const expectedValues = [1, 2, 3]; - const map = new MapPolyfill(); - map.set('a', 1); - map.set('b', 2); - map.set('c', 3); - - const thisArg = { - arg: 'value' - }; - - //intentionally not using lambda to avoid typescript's this context capture - map.forEach(function (value, key) { - expect(this).to.equal(thisArg); - expect(value).to.equal(expectedValues.shift()); - expect(key).to.equal(expectedKeys.shift()); - }, thisArg); - - expect(expectedValues.length).to.equal(0); - expect(expectedKeys.length).to.equal(0); - }); - }); -}); diff --git a/src/operators/groupBy.ts b/src/operators/groupBy.ts index 3ed28a6966..f94e941fea 100644 --- a/src/operators/groupBy.ts +++ b/src/operators/groupBy.ts @@ -3,10 +3,13 @@ import { Subscription } from '../Subscription'; import { Observable } from '../Observable'; import { Operator } from '../Operator'; import { Subject } from '../Subject'; -import { Map } from '../util/Map'; -import { FastMap } from '../util/FastMap'; import { OperatorFunction } from '../interfaces'; +/** Assert that map is present for this operator */ +if (!Map) { + throw new Error('Map not found, please polyfill'); +} + /* tslint:disable:max-line-length */ export function groupBy(keySelector: (value: T) => K): OperatorFunction>; export function groupBy(keySelector: (value: T) => K, elementSelector: void, durationSelector: (grouped: GroupedObservable) => Observable): OperatorFunction>; @@ -144,7 +147,7 @@ class GroupBySubscriber extends Subscriber implements RefCountSubscr let groups = this.groups; if (!groups) { - groups = this.groups = typeof key === 'string' ? new FastMap() : new Map(); + groups = this.groups = new Map>(); } let group = groups.get(key); diff --git a/src/util/FastMap.ts b/src/util/FastMap.ts deleted file mode 100644 index c7bd54a1b7..0000000000 --- a/src/util/FastMap.ts +++ /dev/null @@ -1,30 +0,0 @@ -export class FastMap { - private values: Object = {}; - - delete(key: string): boolean { - this.values[key] = null; - return true; - } - - set(key: string, value: any): FastMap { - this.values[key] = value; - return this; - } - - get(key: string): any { - return this.values[key]; - } - - forEach(cb: (value: any, key: any) => void, thisArg?: any): void { - const values = this.values; - for (let key in values) { - if (values.hasOwnProperty(key) && values[key] !== null) { - cb.call(thisArg, values[key], key); - } - } - } - - clear(): void { - this.values = {}; - } -} \ No newline at end of file diff --git a/src/util/Map.ts b/src/util/Map.ts deleted file mode 100644 index 563017c000..0000000000 --- a/src/util/Map.ts +++ /dev/null @@ -1,4 +0,0 @@ -import { root } from './root'; -import { MapPolyfill } from './MapPolyfill'; - -export const Map = root.Map || (() => MapPolyfill)(); \ No newline at end of file diff --git a/src/util/MapPolyfill.ts b/src/util/MapPolyfill.ts deleted file mode 100644 index 7100f4b191..0000000000 --- a/src/util/MapPolyfill.ts +++ /dev/null @@ -1,43 +0,0 @@ -export class MapPolyfill { - public size = 0; - private _values: any[] = []; - private _keys: any[] = []; - - get(key: any) { - const i = this._keys.indexOf(key); - return i === -1 ? undefined : this._values[i]; - } - - set(key: any, value: any) { - const i = this._keys.indexOf(key); - if (i === -1) { - this._keys.push(key); - this._values.push(value); - this.size++; - } else { - this._values[i] = value; - } - return this; - } - - delete(key: any): boolean { - const i = this._keys.indexOf(key); - if (i === -1) { return false; } - this._values.splice(i, 1); - this._keys.splice(i, 1); - this.size--; - return true; - } - - clear(): void { - this._keys.length = 0; - this._values.length = 0; - this.size = 0; - } - - forEach(cb: Function, thisArg: any): void { - for (let i = 0; i < this.size; i++) { - cb.call(thisArg, this._values[i], this._keys[i]); - } - } -} \ No newline at end of file