chore: use symbols if possible for private properties on exposed objects

fix #172
angular · Sep 9, 2015 · f8a2be5 · f8a2be5
1 parent 50ce9f3
commit f8a2be5
Show file tree

Hide file tree

Showing 6 changed files with 84 additions and 40 deletions.
diff --git a/lib/core.js b/lib/core.js
@@ -1,5 +1,7 @@
 'use strict';
 
+var keys = require('./keys');
+
 function Zone(parentZone, data) {
   var zone = (arguments.length) ? Object.create(parentZone) : this;
 
@@ -121,10 +123,10 @@ Zone.prototype = {
   enqueueTask: function () {},
   dequeueTask: function () {},
   addEventListener: function () {
-    return this._addEventListener.apply(this, arguments);
+    return this[keys.common.addEventListener].apply(this, arguments);
   },
   removeEventListener: function () {
-    return this._removeEventListener.apply(this, arguments);
+    return this[keys.common.removeEventListener].apply(this, arguments);
   }
 };
 

diff --git a/lib/keys.js b/lib/keys.js
@@ -0,0 +1,23 @@
+/**
+ * Creates keys for `private` properties on exposed objects to minimize interactions with other codebases.
+ * The key will be a Symbol if the host supports it; otherwise a prefixed string.
+ */
+if (typeof Symbol !== 'undefined') {
+  function create(name) {
+    return Symbol(name);
+  } 
+} else {
+  function create(name) {
+    return '_zone$' + name;
+  }
+}
+
+var commonKeys = {
+  addEventListener: create('addEventListener'),
+  removeEventListener: create('removeEventListener')
+};
+
+module.exports = {
+  create: create,
+  common: commonKeys
+};
diff --git a/lib/patch/define-property.js b/lib/patch/define-property.js
@@ -1,11 +1,14 @@
 'use strict';
 
+var keys = require('../keys');
+
 // might need similar for object.freeze
 // i regret nothing
 
 var _defineProperty = Object.defineProperty;
 var _getOwnPropertyDescriptor = Object.getOwnPropertyDescriptor;
 var _create = Object.create;
+var unconfigurablesKey = keys.create('unconfigurables');
 
 function apply() {
   Object.defineProperty = function (obj, prop, desc) {
@@ -49,16 +52,16 @@ function _redefineProperty(obj, prop, desc) {
 };
 
 function isUnconfigurable (obj, prop) {
-  return obj && obj.__unconfigurables && obj.__unconfigurables[prop];
+  return obj && obj[unconfigurablesKey] && obj[unconfigurablesKey][prop];
 }
 
 function rewriteDescriptor (obj, prop, desc) {
   desc.configurable = true;
   if (!desc.configurable) {
-    if (!obj.__unconfigurables) {
-      _defineProperty(obj, '__unconfigurables', { writable: true, value: {} });
+    if (!obj[unconfigurablesKey]) {
+      _defineProperty(obj, unconfigurablesKey, { writable: true, value: {} });
     }
-    obj.__unconfigurables[prop] = true;
+    obj[unconfigurablesKey][prop] = true;
   }
   return desc;
 }

diff --git a/lib/patch/mutation-observer.js b/lib/patch/mutation-observer.js
@@ -1,33 +1,39 @@
 'use strict';
 
+var keys = require('../keys');
+
+var originalInstanceKey = keys.create('originalInstance');
+var creationZoneKey = keys.create('creationZone');
+var isActiveKey = keys.create('isActive');
+
 // wrap some native API on `window`
 function patchClass(className) {
   var OriginalClass = global[className];
   if (!OriginalClass) return;
 
   global[className] = function (fn) {
-    this._o = new OriginalClass(global.zone.bind(fn, true));
+    this[originalInstanceKey] = new OriginalClass(global.zone.bind(fn, true));
     // Remember where the class was instantiate to execute the enqueueTask and dequeueTask hooks
-    this._creationZone = global.zone;
+    this[creationZoneKey] = global.zone;
   };
 
   var instance = new OriginalClass(function () {});
 
   global[className].prototype.disconnect = function () {
-    var result = this._o.disconnect.apply(this._o, arguments);
-    if (this._active) {
-      this._creationZone.dequeueTask();
-      this._active = false;
+    var result = this[originalInstanceKey].disconnect.apply(this[originalInstanceKey], arguments);
+    if (this[isActiveKey]) {
+      this[creationZoneKey].dequeueTask();
+      this[isActiveKey] = false;
     }
     return result;
   };
 
   global[className].prototype.observe = function () {
-    if (!this._active) {
-      this._creationZone.enqueueTask();
-      this._active = true;
+    if (!this[isActiveKey]) {
+      this[creationZoneKey].enqueueTask();
+      this[isActiveKey] = true;
     }
-    return this._o.observe.apply(this._o, arguments);
+    return this[originalInstanceKey].observe.apply(this[originalInstanceKey], arguments);
   };
 
   var prop;
@@ -38,19 +44,19 @@ function patchClass(className) {
       }
       if (typeof instance[prop] === 'function') {
         global[className].prototype[prop] = function () {
-          return this._o[prop].apply(this._o, arguments);
+          return this[originalInstanceKey][prop].apply(this[originalInstanceKey], arguments);
         };
       } else {
         Object.defineProperty(global[className].prototype, prop, {
           set: function (fn) {
             if (typeof fn === 'function') {
-              this._o[prop] = global.zone.bind(fn);
+              this[originalInstanceKey][prop] = global.zone.bind(fn);
             } else {
-              this._o[prop] = fn;
+              this[originalInstanceKey][prop] = fn;
             }
           },
           get: function () {
-            return this._o[prop];
+            return this[originalInstanceKey][prop];
           }
         });
       }

diff --git a/lib/patch/property-descriptor.js b/lib/patch/property-descriptor.js
@@ -2,6 +2,7 @@
 
 var webSocketPatch = require('./websocket');
 var utils = require('../utils');
+var keys = require('../keys');
 
 var eventNames = 'copy cut paste abort blur focus canplay canplaythrough change click contextmenu dblclick drag dragend dragenter dragleave dragover dragstart drop durationchange emptied ended input invalid keydown keypress keyup load loadeddata loadedmetadata loadstart message mousedown mouseenter mouseleave mousemove mouseout mouseover mouseup pause play playing progress ratechange reset scroll seeked seeking select show stalled submit suspend timeupdate volumechange waiting mozfullscreenchange mozfullscreenerror mozpointerlockchange mozpointerlockerror error webglcontextrestored webglcontextlost webglcontextcreationerror'.split(' ');
 
@@ -51,6 +52,8 @@ function canPatchViaPropertyDescriptor() {
   return result;
 };
 
+var unboundKey = keys.create('unbound');
+
 // Whenever any event fires, we check the event target and all parents
 // for `onwhatever` properties and replace them with zone-bound functions
 // - Chrome (for now)
@@ -60,9 +63,9 @@ function patchViaCapturingAllTheEvents() {
     document.addEventListener(property, function (event) {
       var elt = event.target, bound;
       while (elt) {
-        if (elt[onproperty] && !elt[onproperty]._unbound) {
+        if (elt[onproperty] && !elt[onproperty][unboundKey]) {
           bound = global.zone.bind(elt[onproperty]);
-          bound._unbound = elt[onproperty];
+          bound[unboundKey] = elt[onproperty];
           elt[onproperty] = bound;
         }
         elt = elt.parentElement;

diff --git a/lib/utils.js b/lib/utils.js
@@ -1,5 +1,7 @@
 'use strict';
 
+var keys = require('./keys');
+
 function bindArguments(args) {
   for (var i = args.length - 1; i >= 0; i--) {
     if (typeof args[i] === 'function') {
@@ -88,9 +90,12 @@ function patchProperties(obj, properties) {
     });
 };
 
+var originalFnKey = keys.create('originalFn');
+var boundFnsKey = keys.create('boundFns');
+
 function patchEventTargetMethods(obj) {
   // This is required for the addEventListener hook on the root zone.
-  obj._addEventListener = obj.addEventListener;
+  obj[keys.common.addEventListener] = obj.addEventListener;
   obj.addEventListener = function (eventName, handler) {
     var fn;
     //Ignore special listeners of IE11 & Edge dev tools, see https://github.com/angular/zone.js/issues/150
@@ -105,30 +110,32 @@ function patchEventTargetMethods(obj) {
         })(handler);
       } else {
         fn = handler;
-      }
+      } 
 
-      handler._fn = fn;
-      handler._bound = handler._bound || {};
-      arguments[1] = handler._bound[eventName] = zone.bind(fn);
+      handler[originalFnKey] = fn;
+      handler[boundFnsKey] = handler[boundFnsKey] || {};
+      arguments[1] = handler[boundFnsKey][eventName] = zone.bind(fn);
     }
     return global.zone.addEventListener.apply(this, arguments);
   };
 
   // This is required for the removeEventListener hook on the root zone.
-  obj._removeEventListener = obj.removeEventListener;
+  obj[keys.common.removeEventListener] = obj.removeEventListener;
   obj.removeEventListener = function (eventName, handler) {
-    if(handler._bound && handler._bound[eventName]) {
-      var _bound = handler._bound;
+    if(handler[boundFnsKey] && handler[boundFnsKey][eventName]) {
+      var _bound = handler[boundFnsKey];
 
       arguments[1] = _bound[eventName];
       delete _bound[eventName];
     }
     var result = global.zone.removeEventListener.apply(this, arguments);
-    global.zone.dequeueTask(handler._fn);
+    global.zone.dequeueTask(handler[originalFnKey]);
     return result;
   };
 };
 
+var originalInstanceKey = keys.create('originalInstance');
+
 // wrap some native API on `window`
 function patchClass(className) {
   var OriginalClass = global[className];
@@ -137,11 +144,11 @@ function patchClass(className) {
   global[className] = function () {
     var a = bindArguments(arguments);
     switch (a.length) {
-      case 0: this._o = new OriginalClass(); break;
-      case 1: this._o = new OriginalClass(a[0]); break;
-      case 2: this._o = new OriginalClass(a[0], a[1]); break;
-      case 3: this._o = new OriginalClass(a[0], a[1], a[2]); break;
-      case 4: this._o = new OriginalClass(a[0], a[1], a[2], a[3]); break;
+      case 0: this[originalInstanceKey] = new OriginalClass(); break;
+      case 1: this[originalInstanceKey] = new OriginalClass(a[0]); break;
+      case 2: this[originalInstanceKey] = new OriginalClass(a[0], a[1]); break;
+      case 3: this[originalInstanceKey] = new OriginalClass(a[0], a[1], a[2]); break;
+      case 4: this[originalInstanceKey] = new OriginalClass(a[0], a[1], a[2], a[3]); break;
       default: throw new Error('what are you even doing?');
     }
   };
@@ -153,19 +160,19 @@ function patchClass(className) {
     (function (prop) {
       if (typeof instance[prop] === 'function') {
         global[className].prototype[prop] = function () {
-          return this._o[prop].apply(this._o, arguments);
+          return this[originalInstanceKey][prop].apply(this[originalInstanceKey], arguments);
         };
       } else {
         Object.defineProperty(global[className].prototype, prop, {
           set: function (fn) {
             if (typeof fn === 'function') {
-              this._o[prop] = global.zone.bind(fn);
+              this[originalInstanceKey][prop] = global.zone.bind(fn);
             } else {
-              this._o[prop] = fn;
+              this[originalInstanceKey][prop] = fn;
             }
           },
           get: function () {
-            return this._o[prop];
+            return this[originalInstanceKey][prop];
           }
         });
       }