From bc258d6f9df9091c2467ac61d5ea6bdf677f38a0 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Tue, 5 Mar 2019 16:12:07 -0800 Subject: [PATCH 1/3] Allow value to merge from previous behavior property declaration. Fixes #5503 --- lib/legacy/class.js | 25 +++++++++++++++++++++++-- test/unit/behaviors.html | 14 +++++++------- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/lib/legacy/class.js b/lib/legacy/class.js index bb4326e370..77eb4fb690 100644 --- a/lib/legacy/class.js +++ b/lib/legacy/class.js @@ -155,6 +155,27 @@ function flattenBehaviors(behaviors, list, exclude) { return list; } +/** + * Copies property descriptors from source to target, overwriting all fields + * of any previous descriptor for a property *except* for `value`, which is + * merged in from the target if it does not exist on the source. + * + * @param {*} target Target properties object + * @param {*} source Source properties object + */ +function mergeProperties(target, source) { + for (const p in source) { + const targetInfo = target[p]; + const value = targetInfo && targetInfo.value; + const sourceInfo = source[p]; + if (sourceInfo.value === undefined && value !== undefined) { + target[p] = Object.assign({value}, sourceInfo); + } else { + target[p] = sourceInfo; + } + } +} + /* Note about construction and extension of legacy classes. [Changed in Q4 2018 to optimize performance.] @@ -227,10 +248,10 @@ function GenerateClassFromInfo(info, Base, behaviors) { const properties = {}; if (behaviorList) { for (let i=0; i < behaviorList.length; i++) { - Object.assign(properties, behaviorList[i].properties); + mergeProperties(properties, behaviorList[i].properties); } } - Object.assign(properties, info.properties); + mergeProperties(properties, info.properties); return properties; } diff --git a/test/unit/behaviors.html b/test/unit/behaviors.html index 33bdc92497..993ab0a3db 100644 --- a/test/unit/behaviors.html +++ b/test/unit/behaviors.html @@ -402,15 +402,15 @@ behaviors: [ { properties: { - foo: { value: true }, - bar: { value: true} + foo: { value: 'a' }, + bar: { value: 'a'} } }, { properties: { - foo: { value: true }, + foo: { value: 'b' }, bar: String, - zot: {value: true} + zot: {value: 'b'} } }, @@ -587,9 +587,9 @@ test('behavior default values can be overridden', function() { const el = fixture('override-default-value'); - assert.notOk(el.foo); - assert.notOk(el.bar); - assert.notOk(el.zot); + assert.equal(el.foo, 'b'); + assert.equal(el.bar, 'a'); + assert.equal(el.zot, 'b'); }); test('readOnly not applied when property was previously observed', function() { From c467c34570afc5ca0be2f55a13fc1b3e642c846e Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Tue, 5 Mar 2019 16:59:22 -0800 Subject: [PATCH 2/3] Use in check rather than undefined. --- lib/legacy/class.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/legacy/class.js b/lib/legacy/class.js index 77eb4fb690..a35c0c5c84 100644 --- a/lib/legacy/class.js +++ b/lib/legacy/class.js @@ -166,10 +166,9 @@ function flattenBehaviors(behaviors, list, exclude) { function mergeProperties(target, source) { for (const p in source) { const targetInfo = target[p]; - const value = targetInfo && targetInfo.value; const sourceInfo = source[p]; - if (sourceInfo.value === undefined && value !== undefined) { - target[p] = Object.assign({value}, sourceInfo); + if (!('value' in sourceInfo) && targetInfo && ('value' in targetInfo)) { + target[p] = Object.assign({value: targetInfo.value}, sourceInfo); } else { target[p] = sourceInfo; } From 62cf9d98a3207fe7d2061b536ea14e69ceb41f57 Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Tue, 5 Mar 2019 17:38:21 -0800 Subject: [PATCH 3/3] Add extra test --- test/unit/behaviors.html | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test/unit/behaviors.html b/test/unit/behaviors.html index 993ab0a3db..6af4b6512b 100644 --- a/test/unit/behaviors.html +++ b/test/unit/behaviors.html @@ -403,14 +403,16 @@ { properties: { foo: { value: 'a' }, - bar: { value: 'a'} + bar: { value: 'a' }, + ziz: { value: 'a' } } }, { properties: { foo: { value: 'b' }, bar: String, - zot: {value: 'b'} + zot: { value: 'b' }, + ziz: { value: 'b' } } }, @@ -418,7 +420,8 @@ properties: { foo: String, - zot: String + zot: String, + ziz: { value: 'c' } } }); @@ -590,6 +593,7 @@ assert.equal(el.foo, 'b'); assert.equal(el.bar, 'a'); assert.equal(el.zot, 'b'); + assert.equal(el.ziz, 'c'); }); test('readOnly not applied when property was previously observed', function() {