From 2e46c6a59f7cb9d5569989c9783570745d37b5a9 Mon Sep 17 00:00:00 2001 From: Thomas Wilburn Date: Thu, 30 Aug 2018 17:30:30 -0500 Subject: [PATCH 1/8] Fix for IE11 issues Unlike other browsers, IE does not check property values when redefining non-configurable properties, and simply throws whether they're the same or not. This sets configurable: true early, since the list overwrites some of its symbols during construction. --- list/list.js | 1 + 1 file changed, 1 insertion(+) diff --git a/list/list.js b/list/list.js index b92c778..dc8d209 100644 --- a/list/list.js +++ b/list/list.js @@ -341,6 +341,7 @@ var eventsProtoSymbols = ("getOwnPropertySymbols" in Object) ? eventsProtoSymbols.forEach(function(sym) { Object.defineProperty(DefineList.prototype, sym, { + configurable: true, enumerable:false, value: define.eventsProto[sym], writable: true From edb2105360fca55ec1591c32e97371f60c805976 Mon Sep 17 00:00:00 2001 From: Thomas Wilburn Date: Fri, 31 Aug 2018 13:10:57 -0500 Subject: [PATCH 2/8] Add configurable to map events as well --- map/map.js | 1 + 1 file changed, 1 insertion(+) diff --git a/map/map.js b/map/map.js index 14e1354..8964e79 100644 --- a/map/map.js +++ b/map/map.js @@ -278,6 +278,7 @@ var eventsProtoSymbols = ("getOwnPropertySymbols" in Object) ? eventsProtoSymbols.forEach(function(sym) { Object.defineProperty(DefineMap.prototype, sym, { + configurable: true, enumerable:false, value: define.eventsProto[sym], writable: true From 0014f99886af16d767776f68624b95c264c0d191 Mon Sep 17 00:00:00 2001 From: Thomas Wilburn Date: Tue, 4 Sep 2018 18:17:47 -0500 Subject: [PATCH 3/8] Do not use iterables in Set constructor --- list/list.js | 5 ++--- map/map.js | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/list/list.js b/list/list.js index dc8d209..b63c8eb 100644 --- a/list/list.js +++ b/list/list.js @@ -706,9 +706,8 @@ var defineListProto = { var ret; if(this._computed && this._computed[key] && this._computed[key].compute) { ret = {}; - ret.valueDependencies = new Set([ - this._computed[key].compute - ]); + ret.valueDependencies = new Set(); + ret.valueDependencies.add(this._computed[key].compute); } return ret; }, diff --git a/map/map.js b/map/map.js index 694b574..cf6ec66 100644 --- a/map/map.js +++ b/map/map.js @@ -238,9 +238,8 @@ var defineMapProto = { var ret; if(this._computed && this._computed[key] && this._computed[key].compute) { ret = {}; - ret.valueDependencies = new Set([ - this._computed[key].compute - ]); + ret.valueDependencies = new Set(); + ret.valueDependencies.add(this._computed[key].compute); } return ret; } From 6fabf474fb06eea2596c20f53cdda1c5566cf4a6 Mon Sep 17 00:00:00 2001 From: Thomas Wilburn Date: Wed, 5 Sep 2018 14:55:10 -0500 Subject: [PATCH 4/8] Make more properties configurable --- can-define.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/can-define.js b/can-define.js index 625d870..8beacde 100644 --- a/can-define.js +++ b/can-define.js @@ -46,12 +46,14 @@ if(process.env.NODE_ENV !== 'production') { if (definition.get) { Object.defineProperty(definition.get, "name", { value: "get "+canReflect.getName(obj) + "."+prop, - writable: true + writable: true, + configurable: true }); } if (definition.set) { Object.defineProperty(definition.set, "name", { - value: "set "+canReflect.getName(obj) + "."+prop + value: "set "+canReflect.getName(obj) + "."+prop, + configurable: true }); } return Object.defineProperty(obj, prop, definition); @@ -271,16 +273,19 @@ define.property = function(typePrototype, prop, definition, dataInitializers, co if (definition.get) { Object.defineProperty(definition.get, "name", { value: canReflect.getName(typePrototype) + "'s " + prop + " getter", + configurable: true }); } if (definition.set) { Object.defineProperty(definition.set, "name", { value: canReflect.getName(typePrototype) + "'s " + prop + " setter", + configurable: true }); } if(isValueResolver(definition)) { Object.defineProperty(definition.value, "name", { value: canReflect.getName(typePrototype) + "'s " + prop + " value", + configurable: true }); } } From ed226184c54db2f27a1e178d2601ffeaa33679d7 Mon Sep 17 00:00:00 2001 From: Thomas Wilburn Date: Wed, 5 Sep 2018 14:56:14 -0500 Subject: [PATCH 5/8] Update test for IE Functions don't have a name property in IE, we shouldn't rely on constant strings for this. Also removed the second test which does exactly the same thing as the first, but changes the name. --- test/test-define-only.js | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/test/test-define-only.js b/test/test-define-only.js index 42967d3..6f71adb 100644 --- a/test/test-define-only.js +++ b/test/test-define-only.js @@ -1216,10 +1216,10 @@ testHelpers.dev.devOnlyTest("Setting a value with only a get() generates a warni QUnit.equal(finishErrorCheck(), 1); }); -testHelpers.dev.devOnlyTest("warn on using a Constructor for small-t type definintions", function() { - QUnit.expect(2); +testHelpers.dev.devOnlyTest("warn on using a Constructor for small-t type definitions", function() { + QUnit.expect(1); - var message = 'can-define: the definition for VM{}.currency uses a constructor for "type". Did you mean "Type"?'; + var message = /can-define: the definition for [\w{}\.]+ uses a constructor for "type"\. Did you mean "Type"\?/; var finishErrorCheck = testHelpers.dev.willWarn(message); function Currency() { @@ -1241,21 +1241,6 @@ testHelpers.dev.devOnlyTest("warn on using a Constructor for small-t type defini QUnit.equal(finishErrorCheck(), 1); - message = 'can-define: the definition for VM2{}.currency uses a constructor for "type". Did you mean "Type"?'; - finishErrorCheck = testHelpers.dev.willWarn(message); - - function VM2() {} - - define(VM2.prototype, { - currency: { - type: Currency, // should be `Type: Currency` - default: function() { - return new Currency({}); - } - } - }); - - QUnit.equal(finishErrorCheck(), 1); }); testHelpers.dev.devOnlyTest("warn with constructor for Value instead of Default (#340)", function() { From 292f368fec42995efaa9d1c43e87aa1b96e86f83 Mon Sep 17 00:00:00 2001 From: Thomas Wilburn Date: Wed, 5 Sep 2018 16:35:07 -0500 Subject: [PATCH 6/8] Work around a hilariously bad enumeration bug --- map/map-test.js | 23 +++++++++++++++++++++++ test/test-define-only.js | 2 +- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/map/map-test.js b/map/map-test.js index a7e892a..3e48cd0 100644 --- a/map/map-test.js +++ b/map/map-test.js @@ -1439,6 +1439,27 @@ QUnit.test("expandos use default type (#383)", function(){ }); QUnit.test("do not enumerate anything other than key properties (#369)", function(){ + // Internet Explorer doesn't correctly skip properties that are non-enumerable + // on the current object, but enumerable on the prototype: + var ancestor = { prop: true }; + var F = function() {}; + F.prototype = ancestor; + var descendant = new F(); + Object.defineProperty(descendant, "prop", { + writable: true, + configurable: true, + enumerable: false, + value: true + }); + var test = {}; + for (var k in descendant) test[k] = descendant[k]; + console.log(test); + debugger; + if (test.prop) { + return QUnit.ok(test.prop, "Browser doesn't correctly skip shadowed enumerable properties") + } + + var Type = DefineMap.extend({ aProp: "string", aMethod: function(){} @@ -1448,6 +1469,8 @@ QUnit.test("do not enumerate anything other than key properties (#369)", functio var props = {}; for(var prop in instance) { + var descriptor = Object.getOwnPropertyDescriptor(instance, prop); + if (!descriptor) descriptor = Object.getOwnPropertyDescriptor(Type.prototype, prop) || {}; props[prop] = true; } QUnit.deepEqual(props,{ diff --git a/test/test-define-only.js b/test/test-define-only.js index 6f71adb..415ea19 100644 --- a/test/test-define-only.js +++ b/test/test-define-only.js @@ -1246,7 +1246,7 @@ testHelpers.dev.devOnlyTest("warn on using a Constructor for small-t type defini testHelpers.dev.devOnlyTest("warn with constructor for Value instead of Default (#340)", function() { QUnit.expect(1); - var message = "can-define: Change the 'Value' definition for VM{}.currency to 'Default'."; + var message = /can-define: Change the 'Value' definition for [\w\.{}]+.currency to 'Default'./; var finishErrorCheck = testHelpers.dev.willWarn(message); function Currency() { From 9f635709cd448c0923ab9057a00b03b6c11422cb Mon Sep 17 00:00:00 2001 From: Thomas Wilburn Date: Wed, 5 Sep 2018 17:17:55 -0500 Subject: [PATCH 7/8] remove debugger statement --- map/map-test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/map/map-test.js b/map/map-test.js index 3e48cd0..7030928 100644 --- a/map/map-test.js +++ b/map/map-test.js @@ -1453,8 +1453,6 @@ QUnit.test("do not enumerate anything other than key properties (#369)", functio }); var test = {}; for (var k in descendant) test[k] = descendant[k]; - console.log(test); - debugger; if (test.prop) { return QUnit.ok(test.prop, "Browser doesn't correctly skip shadowed enumerable properties") } From a942593b029c3fae6aad3c340cb9af21822f1095 Mon Sep 17 00:00:00 2001 From: Thomas Wilburn Date: Fri, 7 Sep 2018 15:42:37 -0500 Subject: [PATCH 8/8] Fix linting errors --- map/map-test.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/map/map-test.js b/map/map-test.js index 7030928..22d74b8 100644 --- a/map/map-test.js +++ b/map/map-test.js @@ -1451,10 +1451,13 @@ QUnit.test("do not enumerate anything other than key properties (#369)", functio enumerable: false, value: true }); + var test = {}; - for (var k in descendant) test[k] = descendant[k]; + for (var k in descendant) { + test[k] = descendant[k]; + } if (test.prop) { - return QUnit.ok(test.prop, "Browser doesn't correctly skip shadowed enumerable properties") + return QUnit.ok(test.prop, "Browser doesn't correctly skip shadowed enumerable properties"); } @@ -1466,9 +1469,7 @@ QUnit.test("do not enumerate anything other than key properties (#369)", functio var instance = new Type({aProp: "VALUE", anExpando: "VALUE"}); var props = {}; - for(var prop in instance) { - var descriptor = Object.getOwnPropertyDescriptor(instance, prop); - if (!descriptor) descriptor = Object.getOwnPropertyDescriptor(Type.prototype, prop) || {}; + for (var prop in instance) { props[prop] = true; } QUnit.deepEqual(props,{