From 1c187d7b02f7d4fc034a984ef270a6197123a7d5 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Thu, 29 Oct 2015 21:59:01 -0700 Subject: [PATCH 1/9] perf(copy): use ES6 Map to track source/destination objects --- src/Angular.js | 58 ++++++++++++++++++++++++++++++++++++++------- test/.eslintrc.json | 2 ++ test/AngularSpec.js | 33 ++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 9 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index ad68d3e128d5..ade4cc2bc456 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -749,6 +749,49 @@ function arrayRemove(array, value) { return index; } + +function ES6MapShim() { + this._keys = []; + this._values = []; +} +ES6MapShim.prototype = { + get: function(key) { + var idx = this._keys.indexOf(key); + if (idx !== -1) { + return this._values[idx]; + } + }, + set: function(key, value) { + var idx = this._keys.indexOf(key); + if (idx === -1) { + idx = this._keys.length; + } + this._keys[idx] = key; + this._values[idx] = value; + return this; + } +}; + +function testES6Map(Map) { + var m, o = {}; + return isFunction(Map) && (m = new Map()) + + // Required functions + && isFunction(m.get) && isFunction(m.set) + + // Number keys, must not call toString + && m.get(1) === undefined + && m.set(1, o) === m + && m.get(1) === o + && m.get('1') === undefined + + // Object keys, must use instance as key and not call toString + && m.set(o, 2) === m && m.get(o) === 2 + && m.get({}) === undefined; +} + +var ES6Map = testES6Map(window.Map) ? window.Map : ES6MapShim; + /** * @ngdoc function * @name angular.copy @@ -815,8 +858,7 @@ function arrayRemove(array, value) { */ function copy(source, destination) { - var stackSource = []; - var stackDest = []; + var stack = new ES6Map(); if (destination) { if (isTypedArray(destination) || isArrayBuffer(destination)) { @@ -837,8 +879,7 @@ function copy(source, destination) { }); } - stackSource.push(source); - stackDest.push(destination); + stack.set(source, destination); return copyRecurse(source, destination); } @@ -882,9 +923,9 @@ function copy(source, destination) { } // Already copied values - var index = stackSource.indexOf(source); - if (index !== -1) { - return stackDest[index]; + var existingCopy = stack.get(source); + if (existingCopy) { + return existingCopy; } if (isWindow(source) || isScope(source)) { @@ -900,8 +941,7 @@ function copy(source, destination) { needsRecurse = true; } - stackSource.push(source); - stackDest.push(destination); + stack.set(source, destination); return needsRecurse ? copyRecurse(source, destination) diff --git a/test/.eslintrc.json b/test/.eslintrc.json index ca882335b492..eb326ac4529a 100644 --- a/test/.eslintrc.json +++ b/test/.eslintrc.json @@ -104,6 +104,8 @@ "getBlockNodes": false, "createMap": false, "VALIDITY_STATE_PROPERTY": true, + "testES6Map": true, + "ES6MapShim": true, /* AngularPublic.js */ "version": false, diff --git a/test/AngularSpec.js b/test/AngularSpec.js index 6fa00567139a..4f05f567be76 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -25,6 +25,39 @@ describe('angular', function() { }); }); + describe('ES6Map', function() { + it('should test for bad Map objects', function() { + expect(testES6Map()).toBe(false); + expect(testES6Map(null)).toBe(false); + expect(testES6Map(3)).toBe(false); + expect(testES6Map({})).toBe(false); + }); + + it('should test for bad Map shims', function() { + function NoMethods() {} + + function ToStringOnKeys() { + this._vals = {}; + } + ToStringOnKeys.prototype = { + get: function(key) { + return this._vals[key]; + }, + set: function(key, val) { + this._vals[key] = val; + return this; + } + }; + + expect(testES6Map(NoMethods)).toBe(false); + expect(testES6Map(ToStringOnKeys)).toBe(false); + }); + + it('should pass the ES6Map test with ES6MapShim', function() { + expect(testES6Map(ES6MapShim)).toBe(true); + }); + }); + describe('copy', function() { it('should return same object', function() { var obj = {}; From 8f4eab4d0e3ff877008ad55a8622fd1d308885af Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Thu, 10 Dec 2015 23:20:07 -0800 Subject: [PATCH 2/9] fixup! perf(copy): Saving the last key/index lookup in ESMapShim --- src/Angular.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index ade4cc2bc456..d8da0a762cd3 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -753,18 +753,27 @@ function arrayRemove(array, value) { function ES6MapShim() { this._keys = []; this._values = []; + this._lastKey = NaN; + this._lastIndex = -1; } ES6MapShim.prototype = { + _idx: function(key) { + if (key === this._lastKey) { + return this._lastIndex; + } + return (this._lastIndex = (this._keys.indexOf(this._lastKey = key))); + }, get: function(key) { - var idx = this._keys.indexOf(key); + var idx = this._idx(key); if (idx !== -1) { return this._values[idx]; } }, set: function(key, value) { - var idx = this._keys.indexOf(key); + var idx = this._idx(key); if (idx === -1) { - idx = this._keys.length; + idx = this._lastIndex = this._keys.length; + this._lastKey = key; } this._keys[idx] = key; this._values[idx] = value; From 95f9179c6e946feb4bc18e5f22488277d25ceca3 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Sat, 27 Aug 2016 11:25:01 -0700 Subject: [PATCH 3/9] fixup! perf(copy): replace Map feature-test with native-test --- src/Angular.js | 30 +++++++++--------------------- test/AngularSpec.js | 33 --------------------------------- 2 files changed, 9 insertions(+), 54 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index d8da0a762cd3..20360e1285e7 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -749,7 +749,9 @@ function arrayRemove(array, value) { return index; } - +// A minimal ES6 Map implementation. +// Should be bug/feature equivelent to the native implementations of supported browsers. +// See https://kangax.github.io/compat-table/es6/#test-Map function ES6MapShim() { this._keys = []; this._values = []; @@ -777,29 +779,15 @@ ES6MapShim.prototype = { } this._keys[idx] = key; this._values[idx] = value; - return this; + + // Support: IE11 + // Do not `return this` to simulate the partial IE11 implementation } }; -function testES6Map(Map) { - var m, o = {}; - return isFunction(Map) && (m = new Map()) - - // Required functions - && isFunction(m.get) && isFunction(m.set) - - // Number keys, must not call toString - && m.get(1) === undefined - && m.set(1, o) === m - && m.get(1) === o - && m.get('1') === undefined - - // Object keys, must use instance as key and not call toString - && m.set(o, 2) === m && m.get(o) === 2 - && m.get({}) === undefined; -} - -var ES6Map = testES6Map(window.Map) ? window.Map : ES6MapShim; +var ES6Map = isFunction(window.Map) && toString.call(window.Map.prototype) === '[object Map]' + ? window.Map + : ES6MapShim; /** * @ngdoc function diff --git a/test/AngularSpec.js b/test/AngularSpec.js index 4f05f567be76..6fa00567139a 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -25,39 +25,6 @@ describe('angular', function() { }); }); - describe('ES6Map', function() { - it('should test for bad Map objects', function() { - expect(testES6Map()).toBe(false); - expect(testES6Map(null)).toBe(false); - expect(testES6Map(3)).toBe(false); - expect(testES6Map({})).toBe(false); - }); - - it('should test for bad Map shims', function() { - function NoMethods() {} - - function ToStringOnKeys() { - this._vals = {}; - } - ToStringOnKeys.prototype = { - get: function(key) { - return this._vals[key]; - }, - set: function(key, val) { - this._vals[key] = val; - return this; - } - }; - - expect(testES6Map(NoMethods)).toBe(false); - expect(testES6Map(ToStringOnKeys)).toBe(false); - }); - - it('should pass the ES6Map test with ES6MapShim', function() { - expect(testES6Map(ES6MapShim)).toBe(true); - }); - }); - describe('copy', function() { it('should return same object', function() { var obj = {}; From 4db3ac12019a98c7f3638e075f5e3df4395da33f Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Sun, 28 Aug 2016 01:17:39 -0700 Subject: [PATCH 4/9] refactor(copy): avoid creating source/dest Map for simple copies --- src/Angular.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index 20360e1285e7..eb982520c51e 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -855,7 +855,7 @@ var ES6Map = isFunction(window.Map) && toString.call(window.Map.prototype) === ' */ function copy(source, destination) { - var stack = new ES6Map(); + var stack; if (destination) { if (isTypedArray(destination) || isArrayBuffer(destination)) { @@ -876,13 +876,14 @@ function copy(source, destination) { }); } - stack.set(source, destination); return copyRecurse(source, destination); } return copyElement(source); function copyRecurse(source, destination) { + (stack || (stack = new ES6Map())).set(source, destination); + var h = destination.$$hashKey; var key; if (isArray(source)) { @@ -920,7 +921,7 @@ function copy(source, destination) { } // Already copied values - var existingCopy = stack.get(source); + var existingCopy = stack && stack.get(source); if (existingCopy) { return existingCopy; } @@ -930,19 +931,15 @@ function copy(source, destination) { 'Can\'t copy! Making copies of Window or Scope instances is not supported.'); } - var needsRecurse = false; var destination = copyType(source); if (destination === undefined) { - destination = isArray(source) ? [] : Object.create(getPrototypeOf(source)); - needsRecurse = true; + destination = copyRecurse(source, isArray(source) ? [] : Object.create(getPrototypeOf(source))); + } else if (stack) { + stack.set(source, destination); } - stack.set(source, destination); - - return needsRecurse - ? copyRecurse(source, destination) - : destination; + return destination; } function copyType(source) { From fdba29734c8a1f99717378a599b777d8ca734697 Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Fri, 9 Sep 2016 16:33:43 +0300 Subject: [PATCH 5/9] refactor(HashMap): use `ES6Map` under the hood --- src/Angular.js | 20 +++++++++-- src/apis.js | 19 +++++----- src/auto/injector.js | 2 +- src/ngMock/angular-mocks.js | 6 ---- test/ApiSpecs.js | 59 ++++++++++++-------------------- test/helpers/testabilityPatch.js | 19 +++++----- test/ng/directive/selectSpec.js | 20 +++++------ test/ngAnimate/animateSpec.js | 2 +- test/ngMock/angular-mocksSpec.js | 17 --------- 9 files changed, 67 insertions(+), 97 deletions(-) diff --git a/src/Angular.js b/src/Angular.js index eb982520c51e..1a5455aa701e 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -89,6 +89,7 @@ hasOwnProperty, createMap, stringify, + ES6Map, NODE_TYPE_ELEMENT, NODE_TYPE_ATTRIBUTE, @@ -752,6 +753,7 @@ function arrayRemove(array, value) { // A minimal ES6 Map implementation. // Should be bug/feature equivelent to the native implementations of supported browsers. // See https://kangax.github.io/compat-table/es6/#test-Map +var nanPlaceholder = Object.create(null); function ES6MapShim() { this._keys = []; this._values = []; @@ -765,23 +767,37 @@ ES6MapShim.prototype = { } return (this._lastIndex = (this._keys.indexOf(this._lastKey = key))); }, + _transformKey: function(key) { + return isNumberNaN(key) ? nanPlaceholder : key; + }, get: function(key) { - var idx = this._idx(key); + var idx = this._idx(this._transformKey(key)); if (idx !== -1) { return this._values[idx]; } }, set: function(key, value) { + key = this._transformKey(key); var idx = this._idx(key); if (idx === -1) { idx = this._lastIndex = this._keys.length; - this._lastKey = key; } this._keys[idx] = key; this._values[idx] = value; // Support: IE11 // Do not `return this` to simulate the partial IE11 implementation + }, + delete: function(key) { + var idx = this._idx(this._transformKey(key)); + if (idx === -1) { + return false; + } + this._keys.splice(idx, 1); + this._values.splice(idx, 1); + this._lastKey = NaN; + this._lastIndex = -1; + return true; } }; diff --git a/src/apis.js b/src/apis.js index 457eb9d1c9b4..caf543b21705 100644 --- a/src/apis.js +++ b/src/apis.js @@ -1,5 +1,7 @@ 'use strict'; +/* global + ES6Map */ /** * Computes a hash of an 'obj'. @@ -36,13 +38,8 @@ function hashKey(obj, nextUidFn) { /** * HashMap which can use objects as keys */ -function HashMap(array, isolatedUid) { - if (isolatedUid) { - var uid = 0; - this.nextUid = function() { - return ++uid; - }; - } +function HashMap(array) { + this._map = new ES6Map(); forEach(array, this.put, this); } HashMap.prototype = { @@ -52,7 +49,7 @@ HashMap.prototype = { * @param value value to store can be any type */ put: function(key, value) { - this[hashKey(key, this.nextUid)] = value; + this._map.set(key, value); }, /** @@ -60,7 +57,7 @@ HashMap.prototype = { * @returns {Object} the value for the key */ get: function(key) { - return this[hashKey(key, this.nextUid)]; + return this._map.get(key); }, /** @@ -68,8 +65,8 @@ HashMap.prototype = { * @param key */ remove: function(key) { - var value = this[key = hashKey(key, this.nextUid)]; - delete this[key]; + var value = this._map.get(key); + this._map.delete(key); return value; } }; diff --git a/src/auto/injector.js b/src/auto/injector.js index 61eac820e035..826a13df6a6e 100644 --- a/src/auto/injector.js +++ b/src/auto/injector.js @@ -649,7 +649,7 @@ function createInjector(modulesToLoad, strictDi) { var INSTANTIATING = {}, providerSuffix = 'Provider', path = [], - loadedModules = new HashMap([], true), + loadedModules = new HashMap(), providerCache = { $provide: { provider: supportObject(provider), diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index 2591716bd998..9c5605f97c85 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -2941,12 +2941,6 @@ angular.mock.$RootScopeDecorator = ['$delegate', function($delegate) { delete fn.$inject; }); - angular.forEach(currentSpec.$modules, function(module) { - if (module && module.$$hashKey) { - module.$$hashKey = undefined; - } - }); - currentSpec.$injector = null; currentSpec.$modules = null; currentSpec.$providerInjector = null; diff --git a/test/ApiSpecs.js b/test/ApiSpecs.js index 349dfe17e662..c5b0fae5dc05 100644 --- a/test/ApiSpecs.js +++ b/test/ApiSpecs.js @@ -5,52 +5,35 @@ describe('api', function() { describe('HashMap', function() { it('should do basic crud', function() { var map = new HashMap(); - var key = {}; - var value1 = {}; - var value2 = {}; - map.put(key, value1); - map.put(key, value2); - expect(map.get(key)).toBe(value2); - expect(map.get({})).toBeUndefined(); - expect(map.remove(key)).toBe(value2); - expect(map.get(key)).toBeUndefined(); + var keys = [{}, {}, {}]; + var values = [{}, {}, {}]; + + map.put(keys[0], values[1]); + map.put(keys[0], values[0]); + expect(map.get(keys[0])).toBe(values[0]); + expect(map.get(keys[1])).toBeUndefined(); + + map.put(keys[1], values[1]); + map.put(keys[2], values[2]); + expect(map.remove(keys[0])).toBe(values[0]); + expect(map.remove(keys[0])).toBeUndefined(); + + expect(map.get(keys[1])).toBe(values[1]); + expect(map.get(keys[2])).toBe(values[2]); }); it('should init from an array', function() { - var map = new HashMap(['a','b']); + var map = new HashMap(['a', 'b']); expect(map.get('a')).toBe(0); expect(map.get('b')).toBe(1); expect(map.get('c')).toBeUndefined(); }); - it('should maintain hashKey for object keys', function() { - var map = new HashMap(); - var key = {}; - map.get(key); - expect(key.$$hashKey).toBeDefined(); - }); - - it('should maintain hashKey for function keys', function() { - var map = new HashMap(); - var key = function() {}; - map.get(key); - expect(key.$$hashKey).toBeDefined(); - }); - - it('should share hashKey between HashMap by default', function() { - var map1 = new HashMap(), map2 = new HashMap(); - var key1 = {}, key2 = {}; - map1.get(key1); - map2.get(key2); - expect(key1.$$hashKey).not.toEqual(key2.$$hashKey); - }); - - it('should maintain hashKey per HashMap if flag is passed', function() { - var map1 = new HashMap([], true), map2 = new HashMap([], true); - var key1 = {}, key2 = {}; - map1.get(key1); - map2.get(key2); - expect(key1.$$hashKey).toEqual(key2.$$hashKey); + it('should init from an object', function() { + var map = new HashMap({one: 'a', two: 'b'}); + expect(map.get('a')).toBe('one'); + expect(map.get('b')).toBe('two'); + expect(map.get('c')).toBeUndefined(); }); }); }); diff --git a/test/helpers/testabilityPatch.js b/test/helpers/testabilityPatch.js index dfb03b31e739..543403e1c85b 100644 --- a/test/helpers/testabilityPatch.js +++ b/test/helpers/testabilityPatch.js @@ -54,17 +54,14 @@ beforeEach(function() { afterEach(function() { var count, cache; - // both of these nodes are persisted across tests - // and therefore the hashCode may be cached - var node = window.document.querySelector('html'); - if (node) { - node.$$hashKey = null; - } - var bod = window.document.body; - if (bod) { - bod.$$hashKey = null; - } - window.document.$$hashKey = null; + // These Nodes are persisted across tests. + // They used to be assigned a `$$hashKey` when animated, but not any more. + var doc = window.document; + var html = doc.querySelector('html'); + var body = doc.body; + expect(doc.$$hashKey).toBeFalsy(); + expect(html && html.$$hashKey).toBeFalsy(); + expect(body && body.$$hashKey).toBeFalsy(); if (this.$injector) { var $rootScope = this.$injector.get('$rootScope'); diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js index a0ecfa71e868..eb04535aa42b 100644 --- a/test/ng/directive/selectSpec.js +++ b/test/ng/directive/selectSpec.js @@ -1515,8 +1515,8 @@ describe('select', function() { 'number:1', 'boolean:true', 'object:null', + 'object:3', 'object:4', - 'object:5', 'number:NaN' ); @@ -1541,7 +1541,7 @@ describe('select', function() { browserTrigger(element, 'change'); var arrayVal = ['a']; - arrayVal.$$hashKey = 'object:5'; + arrayVal.$$hashKey = 'object:4'; expect(scope.selected).toEqual([ 'string', @@ -1549,7 +1549,7 @@ describe('select', function() { 1, true, null, - {prop: 'value', $$hashKey: 'object:4'}, + {prop: 'value', $$hashKey: 'object:3'}, arrayVal, NaN ]); @@ -1862,10 +1862,10 @@ describe('select', function() { scope.$digest(); optionElements = element.find('option'); - expect(element.val()).toBe(prop === 'ngValue' ? 'object:4' : 'C'); + expect(element.val()).toBe(prop === 'ngValue' ? 'object:3' : 'C'); expect(optionElements.length).toEqual(3); expect(optionElements[2].selected).toBe(true); - expect(scope.obj.value).toEqual(prop === 'ngValue' ? {name: 'C', $$hashKey: 'object:4'} : 'C'); + expect(scope.obj.value).toEqual(prop === 'ngValue' ? {name: 'C', $$hashKey: 'object:3'} : 'C'); }); @@ -2174,9 +2174,9 @@ describe('select', function() { expect(optionElements.length).toEqual(4); expect(scope.obj.value).toEqual(prop === 'ngValue' ? [ - {name: 'A', $$hashKey: 'object:4', disabled: true}, - {name: 'C', $$hashKey: 'object:6'}, - {name: 'D', $$hashKey: 'object:7', disabled: true} + {name: 'A', $$hashKey: 'object:3', disabled: true}, + {name: 'C', $$hashKey: 'object:5'}, + {name: 'D', $$hashKey: 'object:6', disabled: true} ] : ['A', 'C', 'D'] ); @@ -2228,13 +2228,13 @@ describe('select', function() { scope.$digest(); optionElements = element.find('option'); - expect(element.val()).toEqual(prop === 'ngValue' ? ['object:4', 'object:5'] : ['B', 'C']); + expect(element.val()).toEqual(prop === 'ngValue' ? ['object:4', 'object:7'] : ['B', 'C']); expect(optionElements.length).toEqual(3); expect(optionElements[1].selected).toBe(true); expect(optionElements[2].selected).toBe(true); expect(scope.obj.value).toEqual(prop === 'ngValue' ? [{ name: 'B', $$hashKey: 'object:4'}, - {name: 'C', $$hashKey: 'object:5'}] : + {name: 'C', $$hashKey: 'object:7'}] : ['B', 'C']); }); diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js index 76ddc34ddabd..bf340e3ff735 100644 --- a/test/ngAnimate/animateSpec.js +++ b/test/ngAnimate/animateSpec.js @@ -1385,7 +1385,7 @@ describe('animations', function() { expect(capturedAnimation[1]).toBe('leave'); // $$hashKey causes comparison issues - expect(element.parent()[0]).toEqual(parent[0]); + expect(element.parent()[0]).toBe(parent[0]); options = capturedAnimation[2]; expect(options.addClass).toEqual('pink'); diff --git a/test/ngMock/angular-mocksSpec.js b/test/ngMock/angular-mocksSpec.js index 134b282515d1..b2d982c61643 100644 --- a/test/ngMock/angular-mocksSpec.js +++ b/test/ngMock/angular-mocksSpec.js @@ -795,23 +795,6 @@ describe('ngMock', function() { }); }); - describe('module cleanup', function() { - function testFn() { - - } - - it('should add hashKey to module function', function() { - module(testFn); - inject(function() { - expect(testFn.$$hashKey).toBeDefined(); - }); - }); - - it('should cleanup hashKey after previous test', function() { - expect(testFn.$$hashKey).toBeUndefined(); - }); - }); - describe('$inject cleanup', function() { function testFn() { From a83f60e144d63175f7f10722b0eb29bc35788cfc Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Fri, 9 Sep 2016 20:39:59 +0300 Subject: [PATCH 6/9] refactor(*): replace `HashMap` with `NgMap` (former `ES6Map`) Also rename: - `ES6Map[Shim]` --> `NgMap[Shim]` - `$$HashMap` --> `$$Map` --- src/.eslintrc.json | 2 +- src/Angular.js | 58 +---------------------- src/AngularPublic.js | 4 +- src/apis.js | 87 ++++++++++++++++++++++------------- src/auto/injector.js | 4 +- src/ng/animate.js | 6 +-- src/ng/directive/select.js | 12 ++--- src/ngAnimate/animateQueue.js | 16 +++---- src/ngAnimate/animation.js | 12 ++--- test/.eslintrc.json | 4 +- test/ApiSpecs.js | 42 +++++++++-------- 11 files changed, 107 insertions(+), 140 deletions(-) diff --git a/src/.eslintrc.json b/src/.eslintrc.json index a5d959535803..db1e99de5119 100644 --- a/src/.eslintrc.json +++ b/src/.eslintrc.json @@ -150,7 +150,7 @@ /* apis.js */ "hashKey": false, - "HashMap": false, + "NgMap": false, /* urlUtils.js */ "urlResolve": false, diff --git a/src/Angular.js b/src/Angular.js index 1a5455aa701e..4435da1d559c 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -89,7 +89,6 @@ hasOwnProperty, createMap, stringify, - ES6Map, NODE_TYPE_ELEMENT, NODE_TYPE_ATTRIBUTE, @@ -750,61 +749,6 @@ function arrayRemove(array, value) { return index; } -// A minimal ES6 Map implementation. -// Should be bug/feature equivelent to the native implementations of supported browsers. -// See https://kangax.github.io/compat-table/es6/#test-Map -var nanPlaceholder = Object.create(null); -function ES6MapShim() { - this._keys = []; - this._values = []; - this._lastKey = NaN; - this._lastIndex = -1; -} -ES6MapShim.prototype = { - _idx: function(key) { - if (key === this._lastKey) { - return this._lastIndex; - } - return (this._lastIndex = (this._keys.indexOf(this._lastKey = key))); - }, - _transformKey: function(key) { - return isNumberNaN(key) ? nanPlaceholder : key; - }, - get: function(key) { - var idx = this._idx(this._transformKey(key)); - if (idx !== -1) { - return this._values[idx]; - } - }, - set: function(key, value) { - key = this._transformKey(key); - var idx = this._idx(key); - if (idx === -1) { - idx = this._lastIndex = this._keys.length; - } - this._keys[idx] = key; - this._values[idx] = value; - - // Support: IE11 - // Do not `return this` to simulate the partial IE11 implementation - }, - delete: function(key) { - var idx = this._idx(this._transformKey(key)); - if (idx === -1) { - return false; - } - this._keys.splice(idx, 1); - this._values.splice(idx, 1); - this._lastKey = NaN; - this._lastIndex = -1; - return true; - } -}; - -var ES6Map = isFunction(window.Map) && toString.call(window.Map.prototype) === '[object Map]' - ? window.Map - : ES6MapShim; - /** * @ngdoc function * @name angular.copy @@ -898,7 +842,7 @@ function copy(source, destination) { return copyElement(source); function copyRecurse(source, destination) { - (stack || (stack = new ES6Map())).set(source, destination); + (stack || (stack = new NgMap())).set(source, destination); var h = destination.$$hashKey; var key; diff --git a/src/AngularPublic.js b/src/AngularPublic.js index 3edd34e3abe9..7f5152c7a7cb 100644 --- a/src/AngularPublic.js +++ b/src/AngularPublic.js @@ -70,7 +70,6 @@ $$ForceReflowProvider, $InterpolateProvider, $IntervalProvider, - $$HashMapProvider, $HttpProvider, $HttpParamSerializerProvider, $HttpParamSerializerJQLikeProvider, @@ -79,6 +78,7 @@ $jsonpCallbacksProvider, $LocationProvider, $LogProvider, + $$MapProvider, $ModelOptionsProvider, $ParseProvider, $RootScopeProvider, @@ -262,7 +262,7 @@ function publishExternalAPI(angular) { $window: $WindowProvider, $$rAF: $$RAFProvider, $$jqLite: $$jqLiteProvider, - $$HashMap: $$HashMapProvider, + $$Map: $$MapProvider, $$cookieReader: $$CookieReaderProvider }); } diff --git a/src/apis.js b/src/apis.js index caf543b21705..21777cab1de0 100644 --- a/src/apis.js +++ b/src/apis.js @@ -1,8 +1,5 @@ 'use strict'; -/* global - ES6Map */ - /** * Computes a hash of an 'obj'. * Hash of a: @@ -35,44 +32,68 @@ function hashKey(obj, nextUidFn) { return key; } -/** - * HashMap which can use objects as keys - */ -function HashMap(array) { - this._map = new ES6Map(); - forEach(array, this.put, this); +// A minimal ES2015 Map implementation. +// Should be bug/feature equivalent to the native implementations of supported browsers +// (for the features required in Angular). +// See https://kangax.github.io/compat-table/es6/#test-Map +var nanKey = Object.create(null); +function NgMapShim() { + this._keys = []; + this._values = []; + this._lastKey = NaN; + this._lastIndex = -1; } -HashMap.prototype = { - /** - * Store key value pair - * @param key key to store can be any type - * @param value value to store can be any type - */ - put: function(key, value) { - this._map.set(key, value); +NgMapShim.prototype = { + _idx: function(key) { + if (key === this._lastKey) { + return this._lastIndex; + } + this._lastKey = key; + this._lastIndex = this._keys.indexOf(key); + return this._lastIndex; + }, + _transformKey: function(key) { + return isNumberNaN(key) ? nanKey : key; }, - - /** - * @param key - * @returns {Object} the value for the key - */ get: function(key) { - return this._map.get(key); + key = this._transformKey(key); + var idx = this._idx(key); + if (idx !== -1) { + return this._values[idx]; + } }, + set: function(key, value) { + key = this._transformKey(key); + var idx = this._idx(key); + if (idx === -1) { + idx = this._lastIndex = this._keys.length; + } + this._keys[idx] = key; + this._values[idx] = value; - /** - * Remove the key/value pair - * @param key - */ - remove: function(key) { - var value = this._map.get(key); - this._map.delete(key); - return value; + // Support: IE11 + // Do not `return this` to simulate the partial IE11 implementation + }, + delete: function(key) { + key = this._transformKey(key); + var idx = this._idx(key); + if (idx === -1) { + return false; + } + this._keys.splice(idx, 1); + this._values.splice(idx, 1); + this._lastKey = NaN; + this._lastIndex = -1; + return true; } }; -var $$HashMapProvider = [/** @this */function() { +var NgMap = isFunction(window.Map) && toString.call(window.Map.prototype) === '[object Map]' + ? window.Map + : NgMapShim; + +var $$MapProvider = [/** @this */function() { this.$get = [function() { - return HashMap; + return NgMap; }]; }]; diff --git a/src/auto/injector.js b/src/auto/injector.js index 826a13df6a6e..4895e356d1ce 100644 --- a/src/auto/injector.js +++ b/src/auto/injector.js @@ -649,7 +649,7 @@ function createInjector(modulesToLoad, strictDi) { var INSTANTIATING = {}, providerSuffix = 'Provider', path = [], - loadedModules = new HashMap(), + loadedModules = new NgMap(), providerCache = { $provide: { provider: supportObject(provider), @@ -757,7 +757,7 @@ function createInjector(modulesToLoad, strictDi) { var runBlocks = [], moduleFn; forEach(modulesToLoad, function(module) { if (loadedModules.get(module)) return; - loadedModules.put(module, true); + loadedModules.set(module, true); function runInvokeQueue(queue) { var i, ii; diff --git a/src/ng/animate.js b/src/ng/animate.js index b55a0376bf4a..c27478e6216a 100644 --- a/src/ng/animate.js +++ b/src/ng/animate.js @@ -60,7 +60,7 @@ var $$CoreAnimateJsProvider = /** @this */ function() { // this is prefixed with Core since it conflicts with // the animateQueueProvider defined in ngAnimate/animateQueue.js var $$CoreAnimateQueueProvider = /** @this */ function() { - var postDigestQueue = new HashMap(); + var postDigestQueue = new NgMap(); var postDigestElements = []; this.$get = ['$$AnimateRunner', '$rootScope', @@ -139,7 +139,7 @@ var $$CoreAnimateQueueProvider = /** @this */ function() { jqLiteRemoveClass(elm, toRemove); } }); - postDigestQueue.remove(element); + postDigestQueue.delete(element); } }); postDigestElements.length = 0; @@ -154,7 +154,7 @@ var $$CoreAnimateQueueProvider = /** @this */ function() { if (classesAdded || classesRemoved) { - postDigestQueue.put(element, data); + postDigestQueue.set(element, data); postDigestElements.push(element); if (postDigestElements.length === 1) { diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index 34f7c8a2a853..b7703cbcef27 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -16,7 +16,7 @@ var SelectController = ['$element', '$scope', /** @this */ function($element, $scope) { var self = this, - optionsMap = new HashMap(); + optionsMap = new NgMap(); self.selectValueMap = {}; // Keys are the hashed values, values the original values @@ -135,7 +135,7 @@ var SelectController = self.emptyOption = element; } var count = optionsMap.get(value) || 0; - optionsMap.put(value, count + 1); + optionsMap.set(value, count + 1); // Only render at the end of a digest. This improves render performance when many options // are added during a digest and ensures all relevant options are correctly marked as selected scheduleRender(); @@ -146,12 +146,12 @@ var SelectController = var count = optionsMap.get(value); if (count) { if (count === 1) { - optionsMap.remove(value); + optionsMap.delete(value); if (value === '') { self.emptyOption = undefined; } } else { - optionsMap.put(value, count - 1); + optionsMap.set(value, count - 1); } } }; @@ -601,9 +601,9 @@ var selectDirective = function() { // Write value now needs to set the selected property of each matching option selectCtrl.writeValue = function writeMultipleValue(value) { - var items = new HashMap(value); forEach(element.find('option'), function(option) { - option.selected = isDefined(items.get(option.value)) || isDefined(items.get(selectCtrl.selectValueMap[option.value])); + option.selected = !!value && (includes(value, option.value) || + includes(value, selectCtrl.selectValueMap[option.value])); }); }; diff --git a/src/ngAnimate/animateQueue.js b/src/ngAnimate/animateQueue.js index bf99c58d6a05..c0de81d4db18 100644 --- a/src/ngAnimate/animateQueue.js +++ b/src/ngAnimate/animateQueue.js @@ -100,15 +100,15 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate return hasMatchingClasses(nA, cR) || hasMatchingClasses(nR, cA); }); - this.$get = ['$$rAF', '$rootScope', '$rootElement', '$document', '$$HashMap', + this.$get = ['$$rAF', '$rootScope', '$rootElement', '$document', '$$Map', '$$animation', '$$AnimateRunner', '$templateRequest', '$$jqLite', '$$forceReflow', '$$isDocumentHidden', - function($$rAF, $rootScope, $rootElement, $document, $$HashMap, + function($$rAF, $rootScope, $rootElement, $document, $$Map, $$animation, $$AnimateRunner, $templateRequest, $$jqLite, $$forceReflow, $$isDocumentHidden) { - var activeAnimationsLookup = new $$HashMap(); - var disabledElementsLookup = new $$HashMap(); + var activeAnimationsLookup = new $$Map(); + var disabledElementsLookup = new $$Map(); var animationsEnabled = null; function postDigestTaskFactory() { @@ -294,7 +294,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate bool = !disabledElementsLookup.get(node); } else { // (element, bool) - Element setter - disabledElementsLookup.put(node, !bool); + disabledElementsLookup.set(node, !bool); } } } @@ -597,7 +597,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate animationDetails.runner.end(); /* falls through */ case PRE_DIGEST_STATE: - activeAnimationsLookup.remove(child); + activeAnimationsLookup.delete(child); break; } } @@ -607,7 +607,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate function clearElementAnimationState(element) { var node = getDomNode(element); node.removeAttribute(NG_ANIMATE_ATTR_NAME); - activeAnimationsLookup.remove(node); + activeAnimationsLookup.delete(node); } function isMatchingElement(nodeOrElmA, nodeOrElmB) { @@ -717,7 +717,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate var newValue = oldValue ? extend(oldValue, details) : details; - activeAnimationsLookup.put(node, newValue); + activeAnimationsLookup.set(node, newValue); } }]; }]; diff --git a/src/ngAnimate/animation.js b/src/ngAnimate/animation.js index 51f104ed7cb8..bd7748114ac2 100644 --- a/src/ngAnimate/animation.js +++ b/src/ngAnimate/animation.js @@ -21,21 +21,21 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro return element.data(RUNNER_STORAGE_KEY); } - this.$get = ['$$jqLite', '$rootScope', '$injector', '$$AnimateRunner', '$$HashMap', '$$rAFScheduler', - function($$jqLite, $rootScope, $injector, $$AnimateRunner, $$HashMap, $$rAFScheduler) { + this.$get = ['$$jqLite', '$rootScope', '$injector', '$$AnimateRunner', '$$Map', '$$rAFScheduler', + function($$jqLite, $rootScope, $injector, $$AnimateRunner, $$Map, $$rAFScheduler) { var animationQueue = []; var applyAnimationClasses = applyAnimationClassesFactory($$jqLite); function sortAnimations(animations) { var tree = { children: [] }; - var i, lookup = new $$HashMap(); + var i, lookup = new $$Map(); - // this is done first beforehand so that the hashmap + // this is done first beforehand so that the map // is filled with a list of the elements that will be animated for (i = 0; i < animations.length; i++) { var animation = animations[i]; - lookup.put(animation.domNode, animations[i] = { + lookup.set(animation.domNode, animations[i] = { domNode: animation.domNode, fn: animation.fn, children: [] @@ -54,7 +54,7 @@ var $$AnimationProvider = ['$animateProvider', /** @this */ function($animatePro var elementNode = entry.domNode; var parentNode = elementNode.parentNode; - lookup.put(elementNode, entry); + lookup.set(elementNode, entry); var parentEntry; while (parentNode) { diff --git a/test/.eslintrc.json b/test/.eslintrc.json index eb326ac4529a..8717139c1bd2 100644 --- a/test/.eslintrc.json +++ b/test/.eslintrc.json @@ -104,8 +104,6 @@ "getBlockNodes": false, "createMap": false, "VALIDITY_STATE_PROPERTY": true, - "testES6Map": true, - "ES6MapShim": true, /* AngularPublic.js */ "version": false, @@ -142,7 +140,7 @@ /* apis.js */ "hashKey": false, - "HashMap": false, + "NgMapShim": false, /* urlUtils.js */ "urlResolve": false, diff --git a/test/ApiSpecs.js b/test/ApiSpecs.js index c5b0fae5dc05..d136ae787f5b 100644 --- a/test/ApiSpecs.js +++ b/test/ApiSpecs.js @@ -2,38 +2,42 @@ describe('api', function() { - describe('HashMap', function() { + describe('NgMapShim', function() { it('should do basic crud', function() { - var map = new HashMap(); + var map = new NgMapShim(); var keys = [{}, {}, {}]; var values = [{}, {}, {}]; - map.put(keys[0], values[1]); - map.put(keys[0], values[0]); + map.set(keys[0], values[1]); + map.set(keys[0], values[0]); expect(map.get(keys[0])).toBe(values[0]); expect(map.get(keys[1])).toBeUndefined(); - map.put(keys[1], values[1]); - map.put(keys[2], values[2]); - expect(map.remove(keys[0])).toBe(values[0]); - expect(map.remove(keys[0])).toBeUndefined(); + map.set(keys[1], values[1]); + map.set(keys[2], values[2]); + expect(map.delete(keys[0])).toBe(true); + expect(map.delete(keys[0])).toBe(false); + expect(map.get(keys[0])).toBeUndefined(); expect(map.get(keys[1])).toBe(values[1]); expect(map.get(keys[2])).toBe(values[2]); }); - it('should init from an array', function() { - var map = new HashMap(['a', 'b']); - expect(map.get('a')).toBe(0); - expect(map.get('b')).toBe(1); - expect(map.get('c')).toBeUndefined(); - }); + it('should be able to deal with `NaN` keys', function() { + var map = new NgMapShim(); + + map.set('NaN', 'foo'); + map.set(NaN, 'bar'); + map.set(NaN, 'baz'); + + expect(map.get('NaN')).toBe('foo'); + expect(map.get(NaN)).toBe('baz'); + + expect(map.delete(NaN)).toBe(true); + expect(map.get(NaN)).toBeUndefined(); + expect(map.get('NaN')).toBe('foo'); - it('should init from an object', function() { - var map = new HashMap({one: 'a', two: 'b'}); - expect(map.get('a')).toBe('one'); - expect(map.get('b')).toBe('two'); - expect(map.get('c')).toBeUndefined(); + expect(map.delete(NaN)).toBe(false); }); }); }); From dcbb7075a2a521f8c87d4d76c4a3cd59d5f1593d Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Fri, 9 Sep 2016 20:42:45 +0300 Subject: [PATCH 7/9] test(hashKey): add tests for `hashKey()` --- test/ApiSpecs.js | 66 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/test/ApiSpecs.js b/test/ApiSpecs.js index d136ae787f5b..bc87caa07105 100644 --- a/test/ApiSpecs.js +++ b/test/ApiSpecs.js @@ -1,6 +1,72 @@ 'use strict'; describe('api', function() { + describe('hashKey()', function() { + it('should use an existing `$$hashKey`', function() { + var obj = {$$hashKey: 'foo'}; + expect(hashKey(obj)).toBe('foo'); + }); + + it('should support a function as `$$hashKey` (and call it)', function() { + var obj = {$$hashKey: valueFn('foo')}; + expect(hashKey(obj)).toBe('foo'); + }); + + it('should create a new `$$hashKey` if none exists (and return it)', function() { + var obj = {}; + expect(hashKey(obj)).toBe(obj.$$hashKey); + expect(obj.$$hashKey).toBeDefined(); + }); + + it('should create appropriate `$$hashKey`s for primitive values', function() { + expect(hashKey(undefined)).toBe(hashKey(undefined)); + expect(hashKey(null)).toBe(hashKey(null)); + expect(hashKey(null)).not.toBe(hashKey(undefined)); + expect(hashKey(true)).toBe(hashKey(true)); + expect(hashKey(false)).toBe(hashKey(false)); + expect(hashKey(false)).not.toBe(hashKey(true)); + expect(hashKey(42)).toBe(hashKey(42)); + expect(hashKey(1337)).toBe(hashKey(1337)); + expect(hashKey(1337)).not.toBe(hashKey(42)); + expect(hashKey('foo')).toBe(hashKey('foo')); + expect(hashKey('foo')).not.toBe(hashKey('bar')); + }); + + it('should create appropriate `$$hashKey`s for non-primitive values', function() { + var fn = function() {}; + var arr = []; + var obj = {}; + var date = new Date(); + + expect(hashKey(fn)).toBe(hashKey(fn)); + expect(hashKey(fn)).not.toBe(hashKey(function() {})); + expect(hashKey(arr)).toBe(hashKey(arr)); + expect(hashKey(arr)).not.toBe(hashKey([])); + expect(hashKey(obj)).toBe(hashKey(obj)); + expect(hashKey(obj)).not.toBe(hashKey({})); + expect(hashKey(date)).toBe(hashKey(date)); + expect(hashKey(date)).not.toBe(hashKey(new Date())); + }); + + it('should support a custom `nextUidFn`', function() { + var nextUidFn = jasmine.createSpy('nextUidFn').and.returnValues('foo', 'bar', 'baz', 'qux'); + + var fn = function() {}; + var arr = []; + var obj = {}; + var date = new Date(); + + hashKey(fn, nextUidFn); + hashKey(arr, nextUidFn); + hashKey(obj, nextUidFn); + hashKey(date, nextUidFn); + + expect(fn.$$hashKey).toBe('function:foo'); + expect(arr.$$hashKey).toBe('object:bar'); + expect(obj.$$hashKey).toBe('object:baz'); + expect(date.$$hashKey).toBe('object:qux'); + }); + }); describe('NgMapShim', function() { it('should do basic crud', function() { From 604d876e7a06976939015e91d2708efe9d60af27 Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Sun, 11 Sep 2016 00:22:56 +0300 Subject: [PATCH 8/9] fixup! refactor(*): replace `HashMap` with `NgMap` (former `ES6Map`) --- src/apis.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/apis.js b/src/apis.js index 21777cab1de0..dd88afbde43c 100644 --- a/src/apis.js +++ b/src/apis.js @@ -88,9 +88,19 @@ NgMapShim.prototype = { } }; -var NgMap = isFunction(window.Map) && toString.call(window.Map.prototype) === '[object Map]' - ? window.Map - : NgMapShim; +// Support: Mobile Safari < 9 +function isNotSafari8BuggyImplementation(Map) { + // Although we don't need the tested feature (`.keys().next()`), its absence indicates a buggy + // implementation, such as the one of [Mobile] Safari 8, which sometimes leads to failures + // (e.g. failing to get a value associated with a jqLite collection). + var m = new Map(); + return isFunction(m.keys) && isFunction(m.keys().next); +} + +var NgMap = isFunction(window.Map) && toString.call(window.Map.prototype) === '[object Map]' && + isNotSafari8BuggyImplementation(window.Map) ? + window.Map : + NgMapShim; var $$MapProvider = [/** @this */function() { this.$get = [function() { From a7f8d630ef52dedc9927b27228ddf6334521828b Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Fri, 9 Dec 2016 14:03:25 +0200 Subject: [PATCH 9/9] fixup! refactor(*): replace `HashMap` with `NgMap` (former `ES6Map`) --- src/apis.js | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/src/apis.js b/src/apis.js index dd88afbde43c..86cbb4a17111 100644 --- a/src/apis.js +++ b/src/apis.js @@ -88,22 +88,11 @@ NgMapShim.prototype = { } }; -// Support: Mobile Safari < 9 -function isNotSafari8BuggyImplementation(Map) { - // Although we don't need the tested feature (`.keys().next()`), its absence indicates a buggy - // implementation, such as the one of [Mobile] Safari 8, which sometimes leads to failures - // (e.g. failing to get a value associated with a jqLite collection). - var m = new Map(); - return isFunction(m.keys) && isFunction(m.keys().next); -} - -var NgMap = isFunction(window.Map) && toString.call(window.Map.prototype) === '[object Map]' && - isNotSafari8BuggyImplementation(window.Map) ? - window.Map : - NgMapShim; - var $$MapProvider = [/** @this */function() { this.$get = [function() { - return NgMap; + // For now, always use `NgMapShim`, even if `window.Map` is available. Some native + // implementations are still buggy (often in subtle ways) and can cause hard-to-debug failures. + // When native `Map` implementations get more stable, we can reconsider. + return NgMapShim; }]; }];