refactor(runtime): Use util.nonEnumerable to define console

[andreubotella]

refactor(runtime): Use util.nonEnumerable to define console

A comment in runtime.js reads that console seems to be "the only one that should be writable and non-enumerable", which explains why it is declared with util.writable but then has its property descriptor's enumerable key changed to false.

But it is not in fact true that console is the only global property for which this holds, and it wasn't even when this behavior was introduced in #9013. All WebIDL interfaces are also writable and non-enumerable – the only difference here being that console is a namespace rather than an interface.

Since WebIDL interfaces are defined with util.nonEnumerable, and console uses the same descriptor keys, this PR changes the definition of console to use util.nonEnumerable as well.

---

Here's the original commit message:

A comment in runtime.js reads that console seems to be "the only one that should be writable and non-enumerable", which explains why it is declared with util.writable to then have the property descriptor's enumerable key changed to false.

That is not in fact true, and it wasn't even in #9013, when that behavior was introduced. In fact all WebIDL interfaces are writable and non-enumerable, declared with util.nonEnumerable – the difference here being that console is a namespace / object.

WebIDL, however, treats namespaces and interfaces similarly, and defines them with the same descriptor keys, so this PR changes the definition of console to use util.nonEnumerable.

[bartlomieju]

Is there any chance we could add some test for this?

[andreubotella]

Is there any chance we could add some test for this?

This is tested as part of console/idlharness.any.html's "console namespace: property descriptor" WPT test: https://github.com/web-platform-tests/wpt/blob/master/resources/idlharness.js#L3258

[bartlomieju]

Is there any chance we could add some test for this?

This is tested as part of console/idlharness.any.html's "console namespace: property descriptor" WPT test: https://github.com/web-platform-tests/wpt/blob/master/resources/idlharness.js#L3258

But is that test run? If so why didn't we catch this inconsistency earlier?

[andreubotella]

But is that test run? If so why didn't we catch this inconsistency earlier?

This isn't an inconsistency. The only difference in the property descriptor between util.writable() and util.nonEnumerable() is that the former sets enumerable to true and the latter to false, so using util.writable() and then setting the enumerable key to false is the same as using util.nonEnumerable().

This change is just a refactoring. But I see that maybe my commit message seemed to imply that there was a mistake. (It is "not in fact true" that console is the only property that is both writable and non-enumerable.) I'll reword that.

[bartlomieju]

LGTM, thank you Andreu