Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Bug 736281 - Implement namespace inheritance. #375

Merged
merged 2 commits into from

3 participants

@Gozala
Owner

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
Owner

@ochameau no rush on reviewing this!

packages/api-utils/lib/message-manager.js
((10 lines not shown))
const Sandbox = require("./sandbox");
+let frame = ns();
@autonome Owner

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
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 Owner

document this function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 Owner

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
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 Owner

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
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 Owner

'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
Owner

@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

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 Owner

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 from
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
  2. @Gozala

    Fix typos.

    Gozala authored
This page is out of date. Refresh to see the latest.
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.