Skip to content

Commit

Permalink
Proxy: Only create scopes when setting values (#8799)
Browse files Browse the repository at this point in the history
  • Loading branch information
kurkle committed Apr 3, 2021
1 parent a4cc21f commit 74f1188
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 8 deletions.
20 changes: 12 additions & 8 deletions src/helpers/helpers.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ import {defined, isArray, isFunction, isObject, resolveObjectKey, _capitalize} f
* @param {string[]} [prefixes] - The prefixes for values, in resolution order.
* @param {object[]} [rootScopes] - The root option scopes
* @param {string|boolean} [fallback] - Parent scopes fallback
* @param {function} [getTarget] - callback for getting the target for changed values
* @returns Proxy
* @private
*/
export function _createResolver(scopes, prefixes = [''], rootScopes = scopes, fallback) {
export function _createResolver(scopes, prefixes = [''], rootScopes = scopes, fallback, getTarget = () => scopes[0]) {
if (!defined(fallback)) {
fallback = _resolve('_fallback', scopes);
}
Expand All @@ -19,6 +20,7 @@ export function _createResolver(scopes, prefixes = [''], rootScopes = scopes, fa
_scopes: scopes,
_rootScopes: rootScopes,
_fallback: fallback,
_getTarget: getTarget,
override: (scope) => _createResolver([scope, ...scopes], prefixes, rootScopes, fallback),
};
return new Proxy(cache, {
Expand Down Expand Up @@ -73,7 +75,8 @@ export function _createResolver(scopes, prefixes = [''], rootScopes = scopes, fa
* A trap for setting property values.
*/
set(target, prop, value) {
scopes[0][prop] = value; // set to top level scope
const storage = target._storage || (target._storage = getTarget());
storage[prop] = value; // set to top level scope
delete target[prop]; // remove from cache
delete target._keys; // remove cached keys
return true;
Expand Down Expand Up @@ -276,11 +279,6 @@ function createSubResolver(parentScopes, resolver, prop, value) {
const fallback = resolveFallback(resolver._fallback, prop, value);
const allScopes = [...parentScopes, ...rootScopes];
const set = new Set();
const firstParent = parentScopes[0];
if (isObject(firstParent) && !(prop in firstParent)) {
// create an empty scope for possible stored values, so we always set the values in top scope.
set.add(firstParent[prop] = {});
}
set.add(value);
let key = addScopesFromKey(set, allScopes, prop, fallback || prop);
if (key === null) {
Expand All @@ -292,7 +290,13 @@ function createSubResolver(parentScopes, resolver, prop, value) {
return false;
}
}
return _createResolver([...set], [''], rootScopes, fallback);
return _createResolver([...set], [''], rootScopes, fallback, () => {
const parent = resolver._getTarget();
if (!(prop in parent)) {
parent[prop] = {};
}
return parent[prop];
});
}

function addScopesFromKey(set, allScopes, key, fallback) {
Expand Down
19 changes: 19 additions & 0 deletions test/specs/helpers.config.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,15 @@ describe('Chart.helpers.config', function() {
expect(resolver.getter).toEqual('options getter');
});

it('should not fail on when options are frozen', function() {
function create() {
const defaults = Object.freeze({default: true});
const options = Object.freeze({value: true});
return _createResolver([options, defaults]);
}
expect(create).not.toThrow();
});

describe('_fallback', function() {
it('should follow simple _fallback', function() {
const defaults = {
Expand Down Expand Up @@ -497,6 +506,16 @@ describe('Chart.helpers.config', function() {
expect(options.sub.value).toBeFalse();
expect(defaults.sub.value).toBeTrue();
});

it('should throw when setting a value and options is frozen', function() {
const defaults = Object.freeze({default: true});
const options = Object.freeze({value: true});
const resolver = _createResolver([options, defaults]);
function set() {
resolver.value = false;
}
expect(set).toThrow();
});
});
});

Expand Down

0 comments on commit 74f1188

Please sign in to comment.