Skip to content

Loading…

Bug 736281 - Implement namespace inheritance. #375

Merged
merged 2 commits into from

3 participants

@Gozala
Mozilla member

I have blogged about namespaces today, where I outlined one annoying limitation that namespaces have in contrast to private names which is this:

let { ns } = require('namespace')
let internals = ns()

let base = {}
internals(base).secret = 'I love JS'

let decedent = Object.create(base)
internals(base).secret // => undefined

With this change last line will return 'I love JS', which I think is more intuitive!

@ochameau ochameau was assigned
@Gozala
Mozilla member

@ochameau no rush on reviewing this!

@autonome autonome commented on an outdated diff
packages/api-utils/lib/message-manager.js
((10 lines not shown))
const Sandbox = require("./sandbox");
+let frame = ns();
@autonome Mozilla member

why 'frame'? also i'd prefer that the name explicitly indicated it's a namespace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@autonome autonome commented on an outdated diff
packages/api-utils/lib/namespace.js
@@ -4,18 +4,38 @@
"use strict";
+const extend = Object.create;
+const proto = Object.getPrototypeOf;
+
+function owns(target, name) {
@autonome Mozilla member

document this function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@autonome autonome commented on an outdated diff
packages/api-utils/lib/namespace.js
((19 lines not shown))
- enumerable: false,
- configurable: false,
- writable: false
- })[handle.key];
+ if (!owns(target, handle.key)) {
+ try {
+ Object.defineProperty(target, handle.key, {
+ value: { '::': target },
+ enumerable: false,
+ configurable: false,
+ writable: false
+ });
+ }
+ // If exception is thrown than object is frozen and we can't add our
+ // special property to it so we just skip to ancestor. This is not perfect
+ // as it may mess up things when two frozen decedents try to use same
@autonome Mozilla member

do you mean 'descendant' instead of 'decedent'?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@autonome autonome commented on an outdated diff
packages/api-utils/lib/namespace.js
((23 lines not shown))
+ if (!owns(target, handle.key)) {
+ try {
+ Object.defineProperty(target, handle.key, {
+ value: { '::': target },
+ enumerable: false,
+ configurable: false,
+ writable: false
+ });
+ }
+ // If exception is thrown than object is frozen and we can't add our
+ // special property to it so we just skip to ancestor. This is not perfect
+ // as it may mess up things when two frozen decedents try to use same
+ // namespace, but this case is pretty rare and this is temporary workaround
+ // anyway so we're should be fine.
+ catch (error) {
+ return handle(Object.getPrototypeOf(target) || {});
@autonome Mozilla member

the caller is getting something that's not what they expect, and have no way of telling that. are you sure we shouldn't just have exception here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@autonome autonome commented on an outdated diff
packages/api-utils/lib/namespace.js
@@ -4,18 +4,38 @@
"use strict";
+const extend = Object.create;
+const proto = Object.getPrototypeOf;
+
+function owns(target, name) {
+ return ~Object.getOwnPropertyNames(target).indexOf(name);
+}
+
// This is a temporary workaround until bug 673468 is fixed, which causes
// entries associated with `XPCWrappedNative` wrapped keys to be GC-ed. To
// workaround that we create a cross reference with an object from the same
// compartment as `WeakMap` and use that as a key. Cross reference prevents
// wrapper to be GC-ed until reference to it's value is kept.
function handle(target) {
@autonome Mozilla member

'handle' doesn't seem to be a very explicit name for what's happening here, but that's not part of this patch, so can change or not as you like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Gozala
Mozilla member

@autonome I have decided to wait until we could remove workaround for bug 673468, as most of the code was doing it. Now that this workarounds can be removed (will be done by #429), I think it's ready for another round. Also keep in mind that in the diff you'll see changes from #429 as well since that's where this branches out from (of course you're welcome to review both). You could see only diff for this patch here https://github.com/Gozala/addon-sdk/compare/bug/ns-workarounds@751584...bug/ns-inherit@736281

@autonome autonome commented on an outdated diff
packages/api-utils/tests/test-namespace.js
((6 lines not shown))
"ns is an alias of Namespace");
-}
+};
+
+exports["test ns inheritance"] = function(assert) {
+ let _ = ns();
+
+ let prototype = { level: 1 };
+ let object = Object.create(prototype);
+ let delegee = Object.create(object);
+
+ _(prototype).foo = {};
+
+ assert.ok(!Object.prototype.hasOwnProperty.call(_(delegee), "foo"),
+ "namespaced property is not copied to decedents");
@autonome Mozilla member

s/decedents/descendants/ here and elsewhere

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Gozala Gozala merged commit ad8c75e into mozilla:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 17, 2012
  1. @Gozala

    Implement namespace inheritance.

    Gozala committed
  2. @Gozala

    Fix typos.

    Gozala committed
Showing with 63 additions and 23 deletions.
  1. +23 −11 packages/api-utils/lib/namespace.js
  2. +40 −12 packages/api-utils/tests/test-namespace.js
View
34 packages/api-utils/lib/namespace.js
@@ -4,24 +4,36 @@
"use strict";
+const create = Object.create;
+const prototypeOf = Object.getPrototypeOf;
+
/**
- * Function creates a new namespace. Optionally `prototype` object may be
- * passed, in which case namespace objects will inherit from it. Returned value
- * is a function that can be used to get access to the namespaced properties
- * for the passed object.
+ * Returns a new namespace, function that may can be used to access an
+ * namespaced object of the argument argument. Namespaced object are associated
+ * with owner objects via weak references. Namespaced objects inherit from the
+ * owners ancestor namespaced object. If owner's ancestor is `null` then
+ * namespaced object inherits from given `prototype`. Namespaces can be used
+ * to define internal APIs that can be shared via enclosing `namespace`
+ * function.
* @examples
- * const ns = Namespace();
- * ns(myObject).secret = secret;
+ * const internals = ns();
+ * internals(object).secret = secret;
*/
-exports.Namespace = function Namespace(prototype) {
- prototype = prototype || Object.prototype;
+function ns() {
const map = new WeakMap();
return function namespace(target) {
- return map.get(target) ||
- map.set(target, Object.create(prototype)), map.get(target);
+ if (!target) // If `target` is not an object return `target` itself.
+ return target;
+ // If target has no namespaced object yet, create one that inherits from
+ // the target prototype's namespaced object.
+ if (!map.has(target))
+ map.set(target, create(namespace(prototypeOf(target) || null)));
+
+ return map.get(target);
};
};
// `Namespace` is a e4x function in the scope, so we export the function also as
// `ns` as alias to avoid clashing.
-exports.ns = exports.Namespace;
+exports.ns = ns;
+exports.Namespace = ns;
View
52 packages/api-utils/tests/test-namespace.js
@@ -4,9 +4,9 @@
"use strict";
-let { Namespace, ns } = require("api-utils/namespace");
-let { Cc, Ci, Cu } = require("chrome");
-let { setTimeout } = require("api-utils/timer")
+const { ns } = require("api-utils/namespace");
+const { Cc, Ci, Cu } = require("chrome");
+const { setTimeout } = require("api-utils/timer")
exports["test post GC references"] = function (assert, done) {
var target = {}, local = ns()
@@ -22,7 +22,7 @@ exports["test post GC references"] = function (assert, done) {
};
exports["test namsepace basics"] = function(assert) {
- var privates = Namespace();
+ var privates = ns();
var object = { foo: function foo() { return "hello foo"; } };
assert.notEqual(privates(object), object,
@@ -35,7 +35,7 @@ exports["test namsepace basics"] = function(assert) {
};
exports["test namespace overlays"] = function(assert) {
- var _ = new Namespace();
+ var _ = ns();
var object = { foo: 'foo' };
_(object).foo = 'bar';
@@ -56,16 +56,17 @@ exports["test namespace overlays"] = function(assert) {
};
exports["test shared namespaces"] = function(assert) {
- var _ = new Namespace({ hello: 'hello world' });
+ var _ = ns();
var f1 = { hello: 1 };
- var f2 = { foo: 'foo' };
+ var f2 = { foo: 'foo', hello: 2 };
+ _(f1).foo = _(f2).foo = 'bar';
assert.equal(_(f1).hello, _(f2).hello, "namespace can be shared");
assert.notEqual(f1.hello, _(f1).hello, "shared namespace can overlay");
assert.notEqual(f2.hello, _(f2).hello, "target is not affected");
- _(f1).hello = 2;
+ _(f1).hello = 3;
assert.notEqual(_(f1).hello, _(f2).hello,
"namespaced property can be overided");
@@ -73,8 +74,8 @@ exports["test shared namespaces"] = function(assert) {
};
exports["test multi namespace"] = function(assert) {
- var n1 = new Namespace();
- var n2 = new Namespace();
+ var n1 = ns();
+ var n2 = ns();
var object = { baz: 1 };
n1(object).foo = 1;
n2(object).foo = 2;
@@ -87,8 +88,35 @@ exports["test multi namespace"] = function(assert) {
};
exports["test ns alias"] = function(assert) {
- assert.strictEqual(ns, Namespace,
+ assert.strictEqual(ns, require('api-utils/namespace').Namespace,
"ns is an alias of Namespace");
-}
+};
+
+exports["test ns inheritance"] = function(assert) {
+ let _ = ns();
+
+ let prototype = { level: 1 };
+ let object = Object.create(prototype);
+ let delegee = Object.create(object);
+
+ _(prototype).foo = {};
+
+ assert.ok(!Object.prototype.hasOwnProperty.call(_(delegee), "foo"),
+ "namespaced property is not copied to descendants");
+ assert.equal(_(delegee).foo, _(prototype).foo,
+ "namespaced properties are inherited by descendants");
+
+ _(object).foo = {};
+ assert.notEqual(_(object).foo, _(prototype).foo,
+ "namespaced properties may be shadowed");
+ assert.equal(_(object).foo, _(delegee).foo,
+ "shadwed properties are inherited by descendants");
+
+ _(object).bar = {};
+ assert.ok(!("bar" in _(prototype)),
+ "descendants properties are not copied to ancestors");
+ assert.ok(_(object).bar, _(delegee).bar,
+ "descendants properties are inherited");
+};
require("test").run(exports);
Something went wrong with that request. Please try again.