[[Prototype]] of namespace objects should be null

[mkubilayk]

• Rollup Version: 1.20.3
• Operating System (or Browser): Mac OS X 10.14.5 18F132
• Node Version: 12.8.0

How Do We Reproduce?

Issue can be seen in the repl.

Expected Behavior

ECMAScript spec defines ES module namespaces as exotic objects and says that their [[Prototype]] should be null.

Actual Behavior

Namespace objects generated by Rollup contains Object.prototype.

Rollup output runs without any errors.

$ npx rollup --input main.mjs --format esm --output.file bundle.mjs && node --experimental-modules bundle.mjs
{ a: [Getter] }
true

If same code is ran without bundling it with Rollup, it produces a TypeError, which is the expected behaviour according to the spec.

$ node --experimental-modules main.mjs 
[Module] { a: 0 }
console.log((0, nsA).hasOwnProperty('a'));
                     ^

TypeError: (0 , nsA).hasOwnProperty is not a function

Using __proto__: null in namespaces would fix this. I would be happy to work on this if you also agree that Rollup output should be as spec compliant as possible.

[lukastaegert]

I'm a little unsure about this one. To my knowledge, messing with __proto__ is a very bad idea. Just read the warnings at the top here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/proto

Warning: Changing the [[Prototype]] of an object is, by the nature of how modern JavaScript engines optimize property accesses, a very slow operation, in every browser and JavaScript engine. The effects on the performance of altering inheritance are subtle and far-flung, and are not limited to simply the time spent in obj.proto = ... statement, but may extend to any code that has access to any object whose [[Prototype]] has been altered. If you care about performance you should avoid setting the [[Prototype]] of an object. Instead, create a new object with the desired [[Prototype]] using Object.create()

That being said, the only safe way of doing this that I am aware of is using Object.create(null). So basically either we do

var ns = /*#__PURE__*/Object.freeze(Object.create(null, {
  x: {value: x, writable: true, enumerable: true}
}));

which quickly becomes really wordy (note that we need the writable: true for the Object.freeze to throw properly...). Alternatively, we assemble the object step by step, though in this case we need to apply a little more work to keep it treeshakeable:

var ns = /*#__PURE__*/(function() {
	var ns = Object.create(null);
	ns.x = x;
	Object.freeze(ns);
	return ns;
} ())

The problem with treeshakeability here is that Rollup does not know that the assignment ns.x = x will not throw an error because it does not know about the return values of Object.create.

I am open to more suggestions but I do not think the either of the approaches are really nice, maybe the second a little nicer than the first. Of course we could also put this behind an option like the namespaceToStringTag.

And please tell me if I miss something here!

[robpalme]

You're right that mutating the prototype of an object after it has been created/used is a bad idea.

In this case, the null prototype is part of the declarative structure of the object from the start. So there is no dynamic mutation going on. { __proto__: null} is a cheaper way of performing Object.create(null) given the former is pure syntax whereas the second involves dynamically looking up a method on Object.

I like your cautiousness though, so will try to get a second opinion from V8 team.

[lukastaegert]

I like your cautiousness though, so will try to get a second opinion from V8 team

That would be awesome. Meanwhile, I keep learning nice hacks such as that { __proto__: null} actually gets rid of the prototype on creation. If there are no concerns from the V8 people, I think this is a nice improvement!

[mathiasbynens]

V8 team member here! I can confirm the nuance that @robpalme pointed out above:

mutating the prototype of an object after it has been created/used is a bad idea.

A more detailed reference is JavaScript engine fundamentals: optimizing prototypes, which concludes:

[…] don’t mess with prototypes (or if you really, really need to, then at least do it before other code runs).

[lukastaegert]

Thanks @mathiasbynens, and nice article by the way. I already started using the pattern

{
  __proto__: null,
  otherProperties: someValues
}

in #3068 because it is just so convenient. I assume that it is still a good idea to put the __proto__: null at the top as the properties are processed top to bottom?

On a side-note, from your article I assume that switching the prototype via obj.__proto = someNewPrototype is just as bad as mutating a prototype, e.g.

const proto = {};
const obj = Object.create(proto);
proto.x = 'foo';

?

[neviaumi]

The PR here (add proto null) made bug when mixed named and default export🤔
https://github.com/snowpackjs/rollup-plugin-polyfill-node/issues/25

Should I just remove named export from pollyfill or I can have flag to control behaviours?