Add namespaces #121
Add namespaces #121
Conversation
|
For |
|
No, I was lazy. I can add it... |
|
👍 Filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=29623 for |
|
Added partial namespaces and fixed a few things I noticed were missing. Left it as a separate commit in case anyone wants to see the sausage being made. |
| <p> | ||
| An ECMAScript implementation would then expose a global property named | ||
| <code>VectorUtils</code> which was a simple object (with prototype | ||
| <a>%ObjectPrototype%</a>) with non-enumerable data properties for each declared |
There was a problem hiding this comment.
This is different from interfaces, where things are enumerable, right? Why the change? Changing the behavior of CSS and console in this way may or may not be web-compatible... (yes, I just checked and all the stuff on console is enumerable in at least Safari, Chrome, and Firefox).
There was a problem hiding this comment.
I was torn on this. Safari indicated a desire to match ES's namespaces (Math, Reflect, JSON) which are not. And, Object.keys(console) currently returns [] because of the prototype separation; this preserves that. To me, it seems more likely people are depending on Object.keys(console) being empty (so that e.g. Object.keys(console) returns any methods they themselves added), than that they are depending on Object.getOwnPropertyDescriptor(console, "log").enumerable being true.
I guess maybe they are using for-in though, which does change.
I should probably flip this.
There was a problem hiding this comment.
for-in is the case that worries me, compat-wise.
|
Updated based on code review comments, including changing to enumerable. |
|
cc @esprehn |
|
@domenic it's hard to tell from the diff; does this PR update http://heycam.github.io/webidl/#dfn-named-definition ? |
|
Also, does it update the
language in http://heycam.github.io/webidl/#idl-names ? |
Yes
Nope, missed that one. Pushed a rebased and squashed commit with that included. The larger question we have before merging and using this for Console is what to do about the fact that console, for legacy reasons, needs an empty [[Prototype]] between it and Object.prototype.
If I had some guidance on that I think we'd be ready to merge and use this... |
|
In Gecko, I implemented an extended attribute for the console thing. How we should spec it.... I'm not sure. :( |
|
I guess I lean toward a one-off requirement in the console spec? In which case this should be mergeable... |
| <div class='note'> | ||
| <p> | ||
| As with partial interface definitions, partial namespace definitions are intended for | ||
| use as a specification editorial aide, allowing the definition of an interface to be |
There was a problem hiding this comment.
"the definition of a namespace"
| namespace and one of its namespace members, then the namespace member's | ||
| <a class='dfnref' href='#dfn-exposure-set'>exposure set</a> | ||
| <span class='rfc2119'>MUST</span> be a subset of the namespace's | ||
| <a class='dfnref' href='#dfn-exposure-set'>exposure set</a>. |
There was a problem hiding this comment.
The spec seems to be missing an obvious requirement for interface that we should add, and add a corresponding one for namespaces: "If [Exposed] appears on a partial interface, then the partial interface's exposure set MUST be a subset of the interface's exposure set." Otherwise we would be allowing this:
interface Foo {
};
[Exposed=Worker]
partial interface Foo {
void func();
};
There was a problem hiding this comment.
I'm not sure how to square that with "Any extended attribute specified on a partial interface definition is considered to appear on the interface itself." It seems like the above snippet somehow makes Foo only exposed in Workers, per that text... (This is similar to my above reply.)
|
Sorry, it wasn't clear to me what the state of this was. I guess if the plan is to add an extended attribute in the console spec that changes the behavior then I'm OK with merging this, modulo the comments above... |
This adds the concept of namespaces, as discussed in whatwg/console#3 (starting especially around whatwg/console#3 (comment)). They can only contain regular operations. The ES binding for namespaces here is written in a fully modern style, and so differs slightly from similar prose for interfaces' ES binding. It is hoped that it can provide a template for eventually updating interfaces' ES binding to modern ES.
- Missing some [SecureContext] stuff - Not yet addressed the operation-creation stuff
It doesn't read quite right to me, but let's keep it as-is for now and investigate any issues in #153.
|
OK, I think everything is addressed; thanks for the careful review. Clearly partial namespaces were added later and I didn't go through and find all the places they needed to be accounted for. The follow-up issues of #154 and #153 are filed, and the "creating an operation function" algorithm is moved into #es-operations with a note about how we'll consolidate soon. I uploaded as separate commits for easier reviewing/verification, but I can squash if @heycam hasn't enabled squashing-from-GitHub-UI on this repo. |
|
OK, I skimmed over the update commits and looks reasonable at first glance. I wish we had a real review system... ;) |
Web IDL added support for namespaces in whatwg/webidl#121 elementSources in css-images-4 is commented out for now because IDL namespaces don't support attributes yet, see w3c#428. Fixes part of https://www.w3.org/Bugs/Public/show_bug.cgi?id=29623
This adds the concept of namespaces, as discussed in whatwg/console#3 (starting especially around whatwg/console#3 (comment)). They can only contain regular operations.
The ES binding for namespaces here is written in a fully modern style, and so differs slightly from similar prose for interfaces' ES binding. It is hoped that it can provide a template for eventually updating interfaces' ES binding to modern ES.