-
Notifications
You must be signed in to change notification settings - Fork 159
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
Switching to Web IDL #963
Comments
I'm in favour, although there are many tricky details. Generally I think the path of least resistance for the standard is likely to also be the path of least resistance for implementations. An exception is options bag processing, where for implementations it is much easier to use dictionaries, but for the standard would require significant reorganisation. I think on balance the benefit of using dictionaries would be worth it, although there may be some costs I haven't identified. Nit: If I understand correctly, the change would mean that some objects would end up being created in different realms? That stuff is super confusing. Hopefully someone from Firefox can comment on this aspect, as I understand they've been suffering from this. I agree on (4). Having internal slots look like member variable accesses and operations look like functions has made it easier for Blink's implementation to match the standard. I wouldn't want to move to a more prosey style. |
Whatever changes you end up making, please consider trying to ensure you preserve the existing fragment IDs. Either that or please make sure to give @whatwg/documentation a heads-up if you do end up needing to break them. The reason I ask that is, MDN docs have already been written for the API, and the MDN articles have links to the existing fragment IDs. So if the fragment IDs change, then all those links break. In general it’d be nice if editors could keep that consideration in mind when making changes. Maybe it would help to think of it in terms of there being a “docs API” that’s bound to the fragment IDs that exist at the time when the docs are written — so if any fragment IDs change, that’s essentially a breaking change to that “docs API” that should be avoided if possible. (P.S. I’m really happy about this actual substance of this planned change.) |
I'm also happy about this; WebIDL is much easier for the MDN writers to figure out in general ;-) |
+1 too. As an implementor, there might be a minor hit (the WebKit implementation might not be as well aligned as with the current spec wording for instance) but I think it is better to move on anyway. |
If this actually happens to be the case, I'm very strongly opposed to this change, for what should be obvious reasons. Moving our implementation out of the JS engine is entirely a non-starter, as it would mean starting from scratch. That said, I do agree WebIDL would otherwise have advantages, so I'll try to figure out a way to keep our implementation while moving to WebIDL. Will update here once I know more. |
WebIDL is only a way of specifying the desired behaviour. APIs that are defined in WebIDL can be implemented in a pure JS engine without the help of a WebIDL-based binding generator; it's just more work. That's already been done for https://webassembly.github.io/spec/js-api/ , for example. Claiming that you'd need to "uproot" any work has no factual basis. [Edited last sentence for tone.] |
Ok, after further discussion with @jorendorff, it seems like we should be able to keep our implementation without too much trouble. I'm personally partial to having non-enumerable methods, but it seems reasonably likely that we'll be able to make them enumerable without too much compat trouble, still. @domenic, one thing that would help is an overview of the behavior changes caused by this. I guess the test changes should uncover most or all of it, so perhaps it's not really needed? Not sure. |
@tschneidereit The wpt tests will start using idlharness.js which is even more pedantic than our existing tests and will quickly identify what you need to change to pass. We've started moving some of Chrome's implementation to WebIDL, so I know some of things that will change.
Some things we should change to make it natural as an IDL spec:
|
In private @tschneidereit mentioned a preference for non-enumerable methods. I'm not really familiar with the pros/cons myself, but as we want to start using IDL for more things, including JavaScript objects, it seems like this needs to be configurable. And if it's configurable we need to pick a correct default going forward. (I'm not sure if the way IDL does argument lengths, stringification, and property lookup is controversial.) I guess those proposing the change to switch streams to IDL don't necessarily want to resolve that question, and I'm not sure it needs to be blocked on it, but I'd like to raise it here so it gets considered. (See also whatwg/webidl#226 and whatwg/webidl#484 for the corresponding issues on the IDL side, and https://github.com/heycam/webidl/labels/jsidl for the general "jsidl" effort, which hasn't really started, but there's still active interest.) |
Glad to hear folks are mostly on-board with this, and that the most contentious issue so far is the non-enumerable vs. enumerable methods ^_^. I don't care much either way there, but I lean toward enumerable since the web has many more enumerable-method classes than JS has non-enumerable-method ones. I think the next step here is for someone to draft up the changes that would be involved. I'll be co-working with @ricea for a few days from the Tokyo office next month, so things might happen around then, but we'll see; we might just work on transferable streams instead. I'll keep folks posted... |
Another difference I noticed:
Re: enumerable vs. non-enumerable, I don't have an opinion except that I'd like to be able to justify whatever choice we make. |
One more difference:
Possible solutions:
I prefer option 2., but I suspect I am in a minority. |
I tend toward 3. It's the more webby thing to do. |
3 seems fine to me, but one thing to note is that if and when we end up adopting WebIDL for JS builtins, we'll need to support non-exposed constructors anyway. ISTM we might as well align streams with that approach. |
How do we express [@@asyncIterator] in WebIDL? Are we blocked on whatwg/webidl#580? |
We are indeed :( |
PR for adding async iterator support to WebIDL: whatwg/webidl#720. |
Closes #963. Normative changes to widely-implemented features, roughly in order of most disruptive to least-disruptive: * For the queuing strategy classes, their size and highWaterMark properties are now getters on the prototype, instead of data properties on the prototype and instance (respectively). Closes #1005. In particular this means that attempts to set either of them post-creation will throw a TypeError. Chromium already ships these semantics. * Functions which take a dictionary no longer accept non-objects. * For the queuing strategy classes, their highWaterMark property will no longer return a non-number from their highWaterMark properties, if one was passed to the constructor. Instead, NaN will be returned. * All methods and accessors are now enumerable, per Web IDL defaults, instead of non-enumerable, per ECMAScript defaults. * All classes are now exposed globally. Formerly, ReadableStreamDefaultReader, ReadableStreamBYOBReader, ReadableStreamDefaultController, ReadableByteStreamController, WritableStreamDefaultWriter, WritableStreamDefaultController, and TransformStreamDefaultController were not exposed. Closes #586. * All classes now have [Symbol.toStringTag] properties. Closes #952. * Some functions have changed their length property value. * Some exceptions are thrown earlier, at argument-conversion time. * Property lookup in options arguments now happens earlier, at argument-conversion time, and in alphabetical order, per dictionary rules. Normative changes to unimplemented features: * ReadableStream's getIterator() method has been renamed to values() as part of adopting Web IDL's infrastructure for async iterators. * The byobRequest property on ReadableByteStreamController now returns null when there is no BYOB request, instead of returning undefined. * The view property on ReadableStreamBYOBRequest now returns null when the view cannot be written into, instead of returning undefined. * Various byte-stream-related APIs that used to specifically prohibit detached buffers now check for zero-length views or buffers, which is a more general category. * The async iterator's next() and return() methods now behave more like async generators, e.g. returning promises fulfilled with { value: undefined, done: true } after return()ing the iterator, instead of returning a rejected promise. Editorial changes: * All APIs are specified to using Web IDL now, instead of using a modified version of the ECMAScript specification conventions. We continue using abstract operations and completion records for now, and we have to drop down to the ECMAScript level in a couple places (notably for dealing with %ObjectPrototype% vs. null-prototype iteration result objects, and transferring array buffers). But overall this removes a lot of type-checking and conversion boilerplate from the specification. Closes #963. Closes #1017. See #1036 for further followup on the iteration result objects. * Individual abstract operations, constructors, methods, and properties no longer have their own heading. They are instead lumped together in sections. Closes #885. * The constructors, methods, and properties are now documented in a per-class block, using the usual WHATWG "domintro" style. Closes #907. * Abstract operations are now consistently alphabetized within their section. Closes #684. * By using Bikeshed's <div algorithm> feature, we now get automatic identifier highlighting. Closes #687. * Switched to 100-character line limits, 1-space indents, and omitting end tags, per WHATWG conventions. * Removed usage of emu-algify in favor of using some more of Bikeshed's built-in features, plus manually annotating a few things. * Switched to concise Bikeshed linking syntax, e.g. [=term=] and [$AbstractOp$]. * Eliminated a number of utility abstract operations, especially around calling functions, by better using Web IDL. Other bug fixes: * Web IDL makes constructor behavior clear, so this closes #965.
This CL adds an explicit [Exposed=(Window,Worker,Worklet)] declaration to the ReadableStreamDefaultController WebIDL file as it was added in a new version of the Streams spec, based on whatwg/streams#963. Intent To Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/yZuxrW9zhAs/m/6bXDaW2mAwAJ Bug: 1093862 Change-Id: I2bd0c759d856c176037231adbe7e679ccea945c8
This CL adds an explicit [Exposed=(Window,Worker,Worklet)] declaration to the ReadableStreamDefaultController WebIDL file as it was added in a new version of the Streams spec, based on whatwg/streams#963. Intent To Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/yZuxrW9zhAs/m/6bXDaW2mAwAJ Bug: 1093862 Change-Id: I2bd0c759d856c176037231adbe7e679ccea945c8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2570755 Reviewed-by: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Commit-Queue: Nidhi Jaju <nidhijaju@google.com> Cr-Commit-Position: refs/heads/master@{#837841}
This CL adds an explicit [Exposed=(Window,Worker,Worklet)] declaration to the ReadableStreamDefaultController WebIDL file as it was added in a new version of the Streams spec, based on whatwg/streams#963. Intent To Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/yZuxrW9zhAs/m/6bXDaW2mAwAJ Bug: 1093862 Change-Id: I2bd0c759d856c176037231adbe7e679ccea945c8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2570755 Reviewed-by: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Commit-Queue: Nidhi Jaju <nidhijaju@google.com> Cr-Commit-Position: refs/heads/master@{#837841}
This CL adds an explicit [Exposed=(Window,Worker,Worklet)] declaration to the ReadableStreamDefaultController WebIDL file as it was added in a new version of the Streams spec, based on whatwg/streams#963. Intent To Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/yZuxrW9zhAs/m/6bXDaW2mAwAJ Bug: 1093862 Change-Id: I2bd0c759d856c176037231adbe7e679ccea945c8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2570755 Reviewed-by: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Commit-Queue: Nidhi Jaju <nidhijaju@google.com> Cr-Commit-Position: refs/heads/master@{#837841}
…DefaultController WebIDL, a=testonly Automatic update from web-platform-tests Add explicit [Exposed] to ReadableStreamDefaultController WebIDL This CL adds an explicit [Exposed=(Window,Worker,Worklet)] declaration to the ReadableStreamDefaultController WebIDL file as it was added in a new version of the Streams spec, based on whatwg/streams#963. Intent To Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/yZuxrW9zhAs/m/6bXDaW2mAwAJ Bug: 1093862 Change-Id: I2bd0c759d856c176037231adbe7e679ccea945c8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2570755 Reviewed-by: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Commit-Queue: Nidhi Jaju <nidhijaju@google.com> Cr-Commit-Position: refs/heads/master@{#837841} -- wpt-commits: 794b4ccac02d86865b5c5d5771c3498d496764dd wpt-pr: 26923
…DefaultController WebIDL, a=testonly Automatic update from web-platform-tests Add explicit [Exposed] to ReadableStreamDefaultController WebIDL This CL adds an explicit [Exposed=(Window,Worker,Worklet)] declaration to the ReadableStreamDefaultController WebIDL file as it was added in a new version of the Streams spec, based on whatwg/streams#963. Intent To Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/yZuxrW9zhAs/m/6bXDaW2mAwAJ Bug: 1093862 Change-Id: I2bd0c759d856c176037231adbe7e679ccea945c8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2570755 Reviewed-by: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Commit-Queue: Nidhi Jaju <nidhijaju@google.com> Cr-Commit-Position: refs/heads/master@{#837841} -- wpt-commits: 794b4ccac02d86865b5c5d5771c3498d496764dd wpt-pr: 26923
…DefaultController WebIDL, a=testonly Automatic update from web-platform-tests Add explicit [Exposed] to ReadableStreamDefaultController WebIDL This CL adds an explicit [Exposed=(Window,Worker,Worklet)] declaration to the ReadableStreamDefaultController WebIDL file as it was added in a new version of the Streams spec, based on whatwg/streams#963. Intent To Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/yZuxrW9zhAs/m/6bXDaW2mAwAJ Bug: 1093862 Change-Id: I2bd0c759d856c176037231adbe7e679ccea945c8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2570755 Reviewed-by: Yoav Weiss <yoavweisschromium.org> Reviewed-by: Yutaka Hirano <yhiranochromium.org> Commit-Queue: Nidhi Jaju <nidhijajugoogle.com> Cr-Commit-Position: refs/heads/master{#837841} -- wpt-commits: 794b4ccac02d86865b5c5d5771c3498d496764dd wpt-pr: 26923 UltraBlame original commit: c266d6f89852334f98b6a02a9a2396be25701c88
…DefaultController WebIDL, a=testonly Automatic update from web-platform-tests Add explicit [Exposed] to ReadableStreamDefaultController WebIDL This CL adds an explicit [Exposed=(Window,Worker,Worklet)] declaration to the ReadableStreamDefaultController WebIDL file as it was added in a new version of the Streams spec, based on whatwg/streams#963. Intent To Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/yZuxrW9zhAs/m/6bXDaW2mAwAJ Bug: 1093862 Change-Id: I2bd0c759d856c176037231adbe7e679ccea945c8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2570755 Reviewed-by: Yoav Weiss <yoavweisschromium.org> Reviewed-by: Yutaka Hirano <yhiranochromium.org> Commit-Queue: Nidhi Jaju <nidhijajugoogle.com> Cr-Commit-Position: refs/heads/master{#837841} -- wpt-commits: 794b4ccac02d86865b5c5d5771c3498d496764dd wpt-pr: 26923 UltraBlame original commit: c266d6f89852334f98b6a02a9a2396be25701c88
…DefaultController WebIDL, a=testonly Automatic update from web-platform-tests Add explicit [Exposed] to ReadableStreamDefaultController WebIDL This CL adds an explicit [Exposed=(Window,Worker,Worklet)] declaration to the ReadableStreamDefaultController WebIDL file as it was added in a new version of the Streams spec, based on whatwg/streams#963. Intent To Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/yZuxrW9zhAs/m/6bXDaW2mAwAJ Bug: 1093862 Change-Id: I2bd0c759d856c176037231adbe7e679ccea945c8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2570755 Reviewed-by: Yoav Weiss <yoavweisschromium.org> Reviewed-by: Yutaka Hirano <yhiranochromium.org> Commit-Queue: Nidhi Jaju <nidhijajugoogle.com> Cr-Commit-Position: refs/heads/master{#837841} -- wpt-commits: 794b4ccac02d86865b5c5d5771c3498d496764dd wpt-pr: 26923 UltraBlame original commit: c266d6f89852334f98b6a02a9a2396be25701c88
Closes whatwg#963. Normative changes to widely-implemented features, roughly in order of most disruptive to least-disruptive: * For the queuing strategy classes, their size and highWaterMark properties are now getters on the prototype, instead of data properties on the prototype and instance (respectively). Closes whatwg#1005. In particular this means that attempts to set either of them post-creation will throw a TypeError. Chromium already ships these semantics. * Functions which take a dictionary no longer accept non-objects. * For the queuing strategy classes, their highWaterMark property will no longer return a non-number from their highWaterMark properties, if one was passed to the constructor. Instead, NaN will be returned. * All methods and accessors are now enumerable, per Web IDL defaults, instead of non-enumerable, per ECMAScript defaults. * All classes are now exposed globally. Formerly, ReadableStreamDefaultReader, ReadableStreamBYOBReader, ReadableStreamDefaultController, ReadableByteStreamController, WritableStreamDefaultWriter, WritableStreamDefaultController, and TransformStreamDefaultController were not exposed. Closes whatwg#586. * All classes now have [Symbol.toStringTag] properties. Closes whatwg#952. * Some functions have changed their length property value. * Some exceptions are thrown earlier, at argument-conversion time. * Property lookup in options arguments now happens earlier, at argument-conversion time, and in alphabetical order, per dictionary rules. Normative changes to unimplemented features: * ReadableStream's getIterator() method has been renamed to values() as part of adopting Web IDL's infrastructure for async iterators. * The byobRequest property on ReadableByteStreamController now returns null when there is no BYOB request, instead of returning undefined. * The view property on ReadableStreamBYOBRequest now returns null when the view cannot be written into, instead of returning undefined. * Various byte-stream-related APIs that used to specifically prohibit detached buffers now check for zero-length views or buffers, which is a more general category. * The async iterator's next() and return() methods now behave more like async generators, e.g. returning promises fulfilled with { value: undefined, done: true } after return()ing the iterator, instead of returning a rejected promise. Editorial changes: * All APIs are specified to using Web IDL now, instead of using a modified version of the ECMAScript specification conventions. We continue using abstract operations and completion records for now, and we have to drop down to the ECMAScript level in a couple places (notably for dealing with %ObjectPrototype% vs. null-prototype iteration result objects, and transferring array buffers). But overall this removes a lot of type-checking and conversion boilerplate from the specification. Closes whatwg#963. Closes whatwg#1017. See whatwg#1036 for further followup on the iteration result objects. * Individual abstract operations, constructors, methods, and properties no longer have their own heading. They are instead lumped together in sections. Closes whatwg#885. * The constructors, methods, and properties are now documented in a per-class block, using the usual WHATWG "domintro" style. Closes whatwg#907. * Abstract operations are now consistently alphabetized within their section. Closes whatwg#684. * By using Bikeshed's <div algorithm> feature, we now get automatic identifier highlighting. Closes whatwg#687. * Switched to 100-character line limits, 1-space indents, and omitting end tags, per WHATWG conventions. * Removed usage of emu-algify in favor of using some more of Bikeshed's built-in features, plus manually annotating a few things. * Switched to concise Bikeshed linking syntax, e.g. [=term=] and [$AbstractOp$]. * Eliminated a number of utility abstract operations, especially around calling functions, by better using Web IDL. Other bug fixes: * Web IDL makes constructor behavior clear, so this closes whatwg#965.
Follows whatwg/streams#963. Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/I3ANo1F_NkM Bug: 1093862 Change-Id: If408bad66762aa66dee9ddc26438ebc9b488d821 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3757032 Commit-Queue: Domenic Denicola <domenic@chromium.org> Reviewed-by: Kent Tamura <tkent@chromium.org> Cr-Commit-Position: refs/heads/main@{#1026650}
This CL adds an explicit [Exposed=(Window,Worker,Worklet)] declaration to the ReadableStreamDefaultController WebIDL file as it was added in a new version of the Streams spec, based on whatwg/streams#963. Intent To Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/yZuxrW9zhAs/m/6bXDaW2mAwAJ Bug: 1093862 Change-Id: I2bd0c759d856c176037231adbe7e679ccea945c8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2570755 Reviewed-by: Yoav Weiss <yoavweiss@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Commit-Queue: Nidhi Jaju <nidhijaju@google.com> Cr-Commit-Position: refs/heads/master@{#837841} GitOrigin-RevId: 970fa27df8cb2a8fa594c07d52b12a73c03114ea
Follows whatwg/streams#963. Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/I3ANo1F_NkM Bug: 1093862 Change-Id: If408bad66762aa66dee9ddc26438ebc9b488d821 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3757032 Commit-Queue: Domenic Denicola <domenic@chromium.org> Reviewed-by: Kent Tamura <tkent@chromium.org> Cr-Commit-Position: refs/heads/main@{#1026650} NOKEYCHECK=True GitOrigin-RevId: aab2e24eaa0ce63a5a2218c3e094970ca8d80d0b
Background
Streams as originally envisioned was meant as a ecosystem-agnostic specification at the level of ECMAScript. As such I wrote it with that specification style (mostly), instead of using Web IDL. See previous discussions in #45 and #732.
Since that time we've seen a shift in the ecosystem, toward using Web IDL more broadly:
Furthermore, not using Web IDL has caused us some pain. I think the pain breaks down in to three main areas:
As such, I think we should reconsider our avoidance of Web IDL. I'd like to use this issue to explore what a switch would look like.
Plan
Concretely, I think there are a few things to consider:
this
value:pipeTo()
(Make pipeThough() non-generic? #961) and the queuing strategysize()
methodshighWaterMark
on the queuing strategy classesSymbol.toStringTag
andtoString
declared? #952any
). Especially options bag processing, when converted to dictionaries.(1) and (2) are the toughest. I would be in favor of just switching wholesale to idiomatic Web IDL, and hoping that there are no web-compat issues. Specifically, moving away from current non-expressible patterns toward more standard ones, and changing argument processing to match how Web IDL dictionaries/optional/etc. work. I think it's likely web-compat will be fine, but who knows, enumerability of methods could throw someone off, or data properties vs. prototypes for the queuing strategies. We could go halfway, e.g. we could add a [NonEnumerable] attribute to Web IDL (it's already in most implementations), or continue defining data properties using ECMAspeak instead of moving
highWaterMark
to be a prototype getter. Thoughts welcome.(3) is just a lot of work. It makes me think we may want to do this piecemeal, e.g. we could convert one class at a time.
(4) is silly and I think we should just keep the current style. Switching is not worth it.
Thoughts?
I'd be interested to hear from various audiences. One thing I'm very concerned about is not making implementers feel like we are creating busywork for them. Or that for those implementers like @tschneidereit who implement streams in the JS engine, they now need to uproot all their work and move it to their rendering engine. I'm hopeful that the precedent of Web Assembly alleviates some of these concerns. So I'd appreciate hearing from folks like @youennf / @othermaciej, @tschneidereit / @jorendorff, @ricea / @yhirano, or @travisleithead about whether they'd feel positively or negatively about this sort of shift.
Thanks all, and my apologies for getting us into this situation so many years ago.
The text was updated successfully, but these errors were encountered: