Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exact normative requirements for interface definitions are unclear #732

Closed
othermaciej opened this issue Apr 12, 2017 · 15 comments
Closed
Labels
editorial Changes that do not affect how the standard is understood

Comments

@othermaciej
Copy link

The Streams standard doesn't use Web IDL to define APIs, unlike many other WHATWG Standards. Instead, it defines interfaces with an introduction like this:

"If one were to write the ReadableStream class in something close to the syntax of [ECMASCRIPT]"

Followed by text that's vaguely similar to ECMAScript code (but in many cases not actually syntactically valid as such).

There doesn't seem to be a definition anywhere of the processing requirements for this quasi-ECMAScript syntax. In the case of Web IDL, all details are precisely defined, including corner cases, and often subtle details have been relevant to interoperability. In theory, an overly literal-minded person could claim that the "if one were to write" sentence imposes no normative requirements at all. In practice, browsers could easily interpret quasi-ECMAScript definitions in subtly different ways that affect interop down the line.

Please either define the quasi-ECMAScript syntax and its processing requirements to a similar level as has been done for Web IDL, or switch to Web IDL instead.

@domenic
Copy link
Member

domenic commented Apr 12, 2017

That section is indeed non-normative. Instead, the normative content is meant to be defined in https://streams.spec.whatwg.org/#conventions, by reference to the ECMAScript specification, which gives very precise definitions to the same level of detail.

@othermaciej
Copy link
Author

As far as I can tell, every other WHATWG Standard that defines APIs uses Web IDL.

@domenic
Copy link
Member

domenic commented Apr 12, 2017

Yep. This one is a bit special since it's (so-far) host agnostic.

@othermaciej
Copy link
Author

@domenic That Conventions section plus interpretive application of the ECMAScript spec does not provide any detail about the detailed behavior of interfaces, for at least the following reasons:

(1) It claims to apply to algorithm definitions not interface definitions.
(2) It doesn't appear to contain any normative requirements, it just defines how terms and conventions are used in the algorithms.
(3) It is mostly devoid of detail on how to apply the ECMAScript spec to something that is merely an interface definition and not an actual class definition.

@domenic
Copy link
Member

domenic commented Apr 12, 2017

I can see how it's not obvious to apply the ECMAScript spec. I can try to tighten that up.

Please keep in mind the informative section you reference is not relevant to the normative content of the spec. The normative content is defined by e.g. the headers, in the manner described in https://tc39.github.io/ecma262/#sec-ecmascript-standard-built-in-objects

@othermaciej
Copy link
Author

OK, I see now that the algorithms define many things in an ECMAScripty way which has more details. I was thrown by the fact that a section labeled "Class Definition" was in fact not a normative definition, my bad.

I hesitate to add more, because I've been told this topic is a "can of worms", but some additional thoughts:

I still think many ECMAScripty details are not fully spelled out. For example, Web IDL spells out what type conversions happen to parameters or values assigned to, how constructors appear on the global object, the full property descriptors of every property, etc. Maybe this can be inferred from "look at the ECMAScript spec" but at a glance I was not sure how.

Web IDL also ensures certain behaviors are consistent. Many browser-hosted implementations of Streams will likely use some form of IDL definition and bindings that are meant to be consistent with Web IDL behaviors. At least Blink and WebKit appear to do it that way. It's somewhat inconvenient that browsers have to each reverse-engineer the IDL from non-IDL definitions. Also, while it may be that what the spec requires happens to 100% align with how the obvious IDL translation would behave, if it doesn't, then it's likely implementations will be wrong. At the very least it's not immediately obvious that this is the case.

Blink and WebKit seem to represent ReadableStream's cancel method like this in IDL (respectively):
[CallWith=ScriptState] Promise cancel(optional any reason);
[NotEnumerable] Promise cancel(optional any reason);

Is either of these correct? I dunno. They both say the reason argument is optional but that is not clear from the spec. The algorithm for the cancel method in the spec does not say it is optional. Maybe it's implicit? But I think other parameters are not optional. I'm not sure how the promise type was determined but it's concerning that they don't agree. I think that difference may result in an observable behavior difference. Also not sure why WebKit decided this method should not be Enumerable.

This is an example of the kinds of details that are left unclear by the current spec style.

@domenic
Copy link
Member

domenic commented Apr 12, 2017

I still think many ECMAScripty details are not fully spelled out. For example, Web IDL spells out what type conversions happen to parameters or values assigned to, how constructors appear on the global object, the full property descriptors of every property, etc.

Like in the ES spec, all conversions are spelled out explicitly in the algorithm steps (e.g. https://streams.spec.whatwg.org/#rs-pipe-to steps 1-3). The global object properties are in https://streams.spec.whatwg.org/#globals.

Blink and WebKit seem to represent ReadableStream's cancel method like this in IDL (respectively):

Where do you find this IDL for Blink? Blink implements its Readable Stream implementation the same way it does its other ES-style builtins: https://cs.chromium.org/webrtc/src/third_party/WebKit/Source/core/streams/ReadableStream.js

I agree that the WebKit engineers seem to have tried to reverse-engineer some Web IDL from the spec's normative text. I'm not sure why they did that; perhaps they found benefits to Web IDL in their pipeline that outweighed the cost of coming up with it themselves (and using nonstandardized extended attributes like [NotEnumerable] to compensate for the IDL/ES mismatch).

Also not sure why WebKit decided this method should not be Enumerable.

(It is in Blink also) This is per https://tc39.github.io/ecma262/#sec-ecmascript-standard-built-in-objects which says

Every other data property described in clauses 18 through 26 and in Annex B.2 has the attributes { [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true } unless otherwise specified.


Stepping back a bit, I acknowledge that the exact pointers to https://tc39.github.io/ecma262/#sec-ecmascript-standard-built-in-objects are not super-clear. And that it's definitely unusual for people used to implementing web specs to understand the ES structure, e.g. the somewhat-hidden "Global Properties" section. The non-normative class definition is certainly misleading and confusing, if read by implementers; it is intended for web developers.

I do believe all details are clear in theory, even if there are practical difficulties in getting to that clarity that I would like to address.

In the end Web IDL might have been a better choice. At the time this spec was first created, it was envisioned as a language-level primitive like promises. It still is envisioned and specced that way, despite it being under WHATWG instead of TC39. But people seem to expect that only TC39 produces specs in the ES style. We didn't realize this expectation would be so strong when creating it.

Now we're in a bit of a tough spot. The spec currently mandates many small things that cannot be expressed in IDL, such as non-enumerable methods, data properties (on the two queuing strategy classes), and no brand-checking (on the queuing strategies and on ReadableStream's pipeThrough method). Additionally, at least two of the open-source implementations (Blink and Gecko) are done without IDL, using the same framework as their JS engine does for other language-level primitives. We could upend all of that; I don't believe the compatibility impact would be too large, as most developers won't be writing code that depends on e.g. countQueuingStrategy.highWaterMark being a data property. But I think it would be a net negative. It would help implementers that use IDL (1/3 open source engines), but make the actual developer-facing semantics slightly less good in various small ways.

@annevk
Copy link
Member

annevk commented Apr 12, 2017

This is a duplicate of #45 I think, though it contains a bit more information.

It also relates to whatwg/webidl#226, which is my quest to do away with differences between JavaScript and IDL classes or at least have the option for uniformity.

@othermaciej
Copy link
Author

@domenic

Where do you find this IDL for Blink? Blink implements its Readable Stream implementation the same way it does its other ES-style builtins: https://cs.chromium.org/webrtc/src/third_party/WebKit/Source/core/streams/ReadableStream.js

I found it here:
https://chromium.googlesource.com/chromium/blink/+/master/Source/core/streams/ReadableStream.idl

I believe that is the canonical Blink repository, as it's referenced in the current Chromium checkout instructions. Your URL seems to be a pointer to a copy of WebKit inside webrtc, and, since it still uses the third_party/WebKit naming, I believe it's older than what I pointed to.

That said, I'm not an expert on Chromium. It's possible that I got the wrong repository, or that I'm looking at a currently unused Streams implementation, or something. If so, my mistake! But even if this implementation is not current, it shows that someone tried to reverse-engineer IDL once.

Every other data property described in clauses 18 through 26 and in Annex B.2 has the attributes { [[Writable]]: true, [[Enumerable]]: false, [[Configurable]]: true } unless otherwise specified.

It was not obvious to me that a statement about ECMASCript specification clauses 18 through 26 and Annex B.2 would apply to content that is not only not in those clauses, but not even in the ECMAScript spec.

Is there any clause in the ECMAScript spec that would let a reader know that the reason parameter is optional? (Or is that a mistake in the IDL translation? Or in the spec?)

@othermaciej
Copy link
Author

@annevk Thanks for the pointers. I'm fine with duping if appropriate. whatwg/webidl#226 makes me sad as I hadn't realized how much Web IDL vs ES-style classes affected behavior. It definitely makes sense to pick one style going forward and to make Web IDL at least optionally support the ES style.

@ricea
Copy link
Collaborator

ricea commented Apr 12, 2017

@othermaciej That's an old copy of the Blink repository from before it was merged with the main Chromium repository. The up-to-date version of that directory is here:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/streams/

Sorry for the confusion.

ReadableStream.idl was removed in November 2015 when we implemented user-constructible ReadableStream objects.

@domenic
Copy link
Member

domenic commented Apr 12, 2017

I believe that is the canonical Blink repository, as it's referenced in the current Chromium checkout instructions. Your URL seems to be a pointer to a copy of WebKit inside webrtc, and, since it still uses the third_party/WebKit naming, I believe it's older than what I pointed to.

Sorry about that. I just grabbed the first ReadableStream.js that I found. The correct one is https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/streams/ReadableStream.js. Adam has explained the confusion about third_party/WebKit inside Chromium vs. Blink

Is there any clause in the ECMAScript spec that would let a reader know that the reason parameter is optional? (Or is that a mistake in the IDL translation? Or in the spec?)

In ES it works the other way around. You have to explicitly choose to throw if something is undefined. Since there is no line "If reason is undefined, throw a TypeError", it is optional. It's all very explicit; no implicit throwing like Web IDL has. Again, this is tricky for people coming into it with expectations from other standards that WHATWG produces, and we didn't realize that there would be such a standards-org <-> assumptions about implicit spec behavior tie when we started this project.

@othermaciej
Copy link
Author

In ES it works the other way around. You have to explicitly choose to throw if something is undefined. Since there is no line "If reason is undefined, throw a TypeError", it is optional.

Does that mean all methods should silently ignore extra arguments instead of throwing, unless explicitly specified otherwise? (Asking because I'm not sure we've implemented this behavior.)

I also think our IDL may have some errors in what is optional and what isn't.

Adam has explained the confusion about third_party/WebKit inside Chromium vs. Blink

I'm still confused about the Blink directory being renamed from blink/ to third_party/WebKit but not in a way that is relevant to this issue.

It may be that, like Blink, we should stop trying to use IDL to implement this spec in WebKit.

@domenic
Copy link
Member

domenic commented Apr 12, 2017

Does that mean all methods should silently ignore extra arguments instead of throwing, unless explicitly specified otherwise?

Yes; that's also what IDL specifies, BTW.

@domenic
Copy link
Member

domenic commented Nov 7, 2018

Let's move this discussion to #963, wherein I contemplate switching to Web IDL.

@domenic domenic closed this as completed Nov 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial Changes that do not affect how the standard is understood
Development

No branches or pull requests

4 participants