refactor(*): replace HashMap with NgMap

For the time being, we will be using `NgMap`, which is an API-compatible
implementation of native `Map` (for the features required in Angular). This will
make it easy to switch to using the native implementations, once they become
more stable.

Note:
At the moment some native implementations are still buggy (often in subtle ways)
and can cause hard-to-debug failures.)

Closes #15483
This commit is contained in:
Georgios Kalpakas
2016-12-09 14:24:00 +02:00
parent 028fa1abb2
commit 641e13acc1
16 changed files with 141 additions and 160 deletions
+1 -1
View File
@@ -150,7 +150,7 @@
/* apis.js */
"hashKey": false,
"HashMap": false,
"NgMap": false,
/* urlUtils.js */
"urlResolve": false,
+2 -2
View File
@@ -70,7 +70,6 @@
$$ForceReflowProvider,
$InterpolateProvider,
$IntervalProvider,
$$HashMapProvider,
$HttpProvider,
$HttpParamSerializerProvider,
$HttpParamSerializerJQLikeProvider,
@@ -79,6 +78,7 @@
$jsonpCallbacksProvider,
$LocationProvider,
$LogProvider,
$$MapProvider,
$ParseProvider,
$RootScopeProvider,
$QProvider,
@@ -260,7 +260,7 @@ function publishExternalAPI(angular) {
$window: $WindowProvider,
$$rAF: $$RAFProvider,
$$jqLite: $$jqLiteProvider,
$$HashMap: $$HashMapProvider,
$$Map: $$MapProvider,
$$cookieReader: $$CookieReaderProvider
});
}
+55 -36
View File
@@ -1,6 +1,5 @@
'use strict';
/**
* Computes a hash of an 'obj'.
* Hash of a:
@@ -33,49 +32,69 @@ function hashKey(obj, nextUidFn) {
return key;
}
/**
* HashMap which can use objects as keys
*/
function HashMap(array, isolatedUid) {
if (isolatedUid) {
var uid = 0;
this.nextUid = function() {
return ++uid;
};
}
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[hashKey(key, this.nextUid)] = 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[hashKey(key, this.nextUid)];
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[key = hashKey(key, this.nextUid)];
delete this[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() {
// 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 switching to `window.Map` (when available).
var NgMap = NgMapShim;
var $$MapProvider = [/** @this */function() {
this.$get = [function() {
return HashMap;
return NgMap;
}];
}];
+2 -2
View File
@@ -649,7 +649,7 @@ function createInjector(modulesToLoad, strictDi) {
var INSTANTIATING = {},
providerSuffix = 'Provider',
path = [],
loadedModules = new HashMap([], true),
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;
+3 -3
View File
@@ -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) {
+6 -6
View File
@@ -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
@@ -137,7 +137,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();
@@ -148,13 +148,13 @@ var SelectController =
var count = optionsMap.get(value);
if (count) {
if (count === 1) {
optionsMap.remove(value);
optionsMap.delete(value);
if (value === '') {
self.hasEmptyOption = false;
self.emptyOption = undefined;
}
} else {
optionsMap.put(value, count - 1);
optionsMap.set(value, count - 1);
}
}
};
@@ -606,9 +606,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]));
});
};
+8 -8
View File
@@ -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() {
@@ -291,7 +291,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
bool = !disabledElementsLookup.get(node);
} else {
// (element, bool) - Element setter
disabledElementsLookup.put(node, !bool);
disabledElementsLookup.set(node, !bool);
}
}
}
@@ -599,7 +599,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
animationDetails.runner.end();
/* falls through */
case PRE_DIGEST_STATE:
activeAnimationsLookup.remove(child);
activeAnimationsLookup.delete(child);
break;
}
}
@@ -608,7 +608,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
function clearElementAnimationState(node) {
node.removeAttribute(NG_ANIMATE_ATTR_NAME);
activeAnimationsLookup.remove(node);
activeAnimationsLookup.delete(node);
}
/**
@@ -713,7 +713,7 @@ var $$AnimateQueueProvider = ['$animateProvider', /** @this */ function($animate
var newValue = oldValue
? extend(oldValue, details)
: details;
activeAnimationsLookup.put(node, newValue);
activeAnimationsLookup.set(node, newValue);
}
}];
}];
+6 -6
View File
@@ -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) {
-6
View File
@@ -2985,12 +2985,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;
+1 -1
View File
@@ -143,7 +143,7 @@
/* apis.js */
"hashKey": false,
"HashMap": false,
"NgMapShim": false,
/* urlUtils.js */
"urlResolve": false,
+29 -42
View File
@@ -68,55 +68,42 @@ describe('api', function() {
});
});
describe('HashMap', function() {
describe('NgMapShim', 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 map = new NgMapShim();
var keys = [{}, {}, {}];
var values = [{}, {}, {}];
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.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();
it('should maintain hashKey for object keys', function() {
var map = new HashMap();
var key = {};
map.get(key);
expect(key.$$hashKey).toBeDefined();
});
map.set('NaN', 'foo');
map.set(NaN, 'bar');
map.set(NaN, 'baz');
it('should maintain hashKey for function keys', function() {
var map = new HashMap();
var key = function() {};
map.get(key);
expect(key.$$hashKey).toBeDefined();
});
expect(map.get('NaN')).toBe('foo');
expect(map.get(NaN)).toBe('baz');
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);
});
expect(map.delete(NaN)).toBe(true);
expect(map.get(NaN)).toBeUndefined();
expect(map.get('NaN')).toBe('foo');
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);
expect(map.delete(NaN)).toBe(false);
});
});
});
+7 -8
View File
@@ -46,14 +46,13 @@ describe('injector', function() {
it('should resolve dependency graph and instantiate all services just once', function() {
var log = [];
// s1
// / | \
// / s2 \
// / / | \ \
// /s3 < s4 > s5
// //
// s6
// s1
// / | \
// / s2 \
// / / | \ \
// /s3 < s4 > s5
// //
// s6
providers('s1', function() { log.push('s1'); return {}; }, {$inject: ['s2', 's5', 's6']});
providers('s2', function() { log.push('s2'); return {}; }, {$inject: ['s3', 's4', 's5']});
+10 -11
View File
@@ -54,17 +54,16 @@ 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, which we needed to clear after each test
// to avoid affecting other tests. This is no longer the case, so we are just ensuring that there
// is indeed no `$$hachKey` on them.
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');
+10 -10
View File
@@ -1529,8 +1529,8 @@ describe('select', function() {
'number:1',
'boolean:true',
'object:null',
'object:3',
'object:4',
'object:5',
'number:NaN'
);
@@ -1555,7 +1555,7 @@ describe('select', function() {
browserTrigger(element, 'change');
var arrayVal = ['a'];
arrayVal.$$hashKey = 'object:5';
arrayVal.$$hashKey = 'object:4';
expect(scope.selected).toEqual([
'string',
@@ -1563,7 +1563,7 @@ describe('select', function() {
1,
true,
null,
{prop: 'value', $$hashKey: 'object:4'},
{prop: 'value', $$hashKey: 'object:3'},
arrayVal,
NaN
]);
@@ -1876,10 +1876,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');
});
@@ -2188,9 +2188,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']
);
@@ -2242,13 +2242,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']);
});
+1 -1
View File
@@ -1407,7 +1407,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');
-17
View File
@@ -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() {