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

feat: Add [NonEnumerableMethods] extended attribute #825

Conversation

ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented Dec 11, 2019

This adds a [NonEnumerableMethods] extended attribute to let specs match what ECMAScript does with its built‑ins and the class construct.

It also provides [LegacyEnumerableMethod] for the legacy cases that would break if the methods on which it’s specified would be made non‑enumerable.


This is need to address #226, #484, WebAssembly/spec#1110 and whatwg/streams#963,

Supersedes and closes #719


Preview | Diff

@bzbarsky
Copy link
Collaborator

bzbarsky commented Dec 11, 2019

A few questions:

  1. This only applies to operations, not attributes? We want attributes to keep being enumerable? What about constants?
  2. It looks like the actual processing model changes needed to make these extended attributes do something are missing. In particular https://heycam.github.io/webidl/#define-the-operations presumably needs to be modified to not always do [[Enumerable]]: true and instead compute that value on a per-operation basis based on these extended attributes.

@ExE-Boss
Copy link
Contributor Author

  1. This only applies to operations, not attributes? We want attributes to keep being enumerable? What about constants?

This only affects operations, everything else is unaffected.

  1. It looks like the actual processing model changes needed to make these extended attributes do something are missing.

You’re right, I missed that.

@bzbarsky
Copy link
Collaborator

More questions:

A few questions:

  1. What happens if a mixin is mixed into interfaces with different enumerability behavior? Presumably the processing model changes will pin this down.
  2. If an operation is inside a mixin that is mixed into both interfaces that are [NonEnumerableMethods] and ones that are not, can that operation be [LegacyEnumerableMethod]? This is entirely unclear, given the language about how that attr most not be used outside of [NonEnumerableMethods] interfaces.
  3. What should happen with stringifiers and their enumerability? That's not covered by <https://heycam.github.io/webidl/#define-the-operations > but specified separately in https://heycam.github.io/webidl/#es-stringifier.
  4. Similar for the iterable forEach stuff defined by https://heycam.github.io/webidl/#es-forEach. And other iterable-induced methods.
  5. Similar for iterator prototype .next methods. This is complicated by the fact that there is no explicit syntactic interface declaration for the iterator stuff.

@ExE-Boss
Copy link
Contributor Author

ExE-Boss commented Dec 11, 2019

  1. What happens if a mixin is mixed into interfaces with different enumerability behavior? Presumably the processing model changes will pin this down.
  2. If an operation is inside a mixin that is mixed into both interfaces that are [NonEnumerableMethods] and ones that are not, can that operation be [LegacyEnumerableMethod]? This is entirely unclear, given the language about how that attr most not be used outside of [NonEnumerableMethods] interfaces.

I would expect that if [NonEnumerableMethods] is defined on the interface mixin, then it would only affect methods defined on that mixin, similarly methods transcluded from mixins [NonEnumerableMethods] would be kept enumerable, but I don't really know how all the moving parts of WebIDL interact with each other, so I don’t know which should be the right behaviour.

  1. What should happen with stringifiers and their enumerability?
  2. Similar for the iterable forEach stuff defined by https://heycam.github.io/webidl/#es-forEach. And other iterable-induced methods.
  3. Similar for iterator prototype .next methods. This is complicated by the fact that there is no explicit syntactic interface declaration for the iterator stuff.

This should probably depend on whether or not [NonEnumerableMethods] is specified on the interface.

@domenic
Copy link
Member

domenic commented Dec 11, 2019

What is the motivation for this feature? In particular, what specs want to take on a different behavior from the rest of the web platform by using this?

@ExE-Boss
Copy link
Contributor Author

What is the motivation for this feature?

To make it possible for specifications using WebIDL to align more closely with how ECMAScript platform objects behave.

In particular, what specs want to take on a different behavior from the rest of the web platform by using this?

ECMAScript and Streams come to mind (you even opened whatwg/streams#963)

It’s also probable that WebAssembly might want to use this.

@domenic
Copy link
Member

domenic commented Dec 11, 2019

ECMAScript

I'd like to see some evidence of TC39 wanting to use Web IDL for ECMAScript before merging this

Streams

We will make all methods enumerable

WebAssembly

I don't see why they would change away from their current approach of aligning with all other web APIs.

@ExE-Boss
Copy link
Contributor Author

ExE-Boss commented Dec 11, 2019

ECMAScript

I'd like to see some evidence of TC39 wanting to use Web IDL for ECMAScript before merging this

There is https://github.com/tc39/proposal-idl, which has WebIDL as one of the main candidates.

WebAssembly

I don't see why they would change away from their current approach of aligning with all other web APIs.

WebAssembly intends to be more of an ECMAScript platform feature, so I would expect its built‑ins to match ECMAScript conventions.

I’ve opened WebAssembly/spec#1110 to ask about this.

@Ms2ger
Copy link
Member

Ms2ger commented Dec 12, 2019

Please explain the difference between this and #719.

@ExE-Boss
Copy link
Contributor Author

Please explain the difference between this and #719.

This also provides the [LegacyEnumerableMethod] extended attribute and defines how stringifiers and other dynamically created methods are affected.

@ExE-Boss
Copy link
Contributor Author

@domenic

Streams

We will make all methods enumerable

Where was that decided? Because I think enumerable methods are bad.

@domenic
Copy link
Member

domenic commented Dec 14, 2019

I think they're good. (I don't anticipate this being a productive discussion, so don't plan to engage further.)

@ExE-Boss
Copy link
Contributor Author

@domenic That doesn’t answer my question about where it was decided to change methods in the Streams API to be enumerable.

@ExE-Boss
Copy link
Contributor Author

ExE-Boss commented Jan 4, 2020

This should maybe have the jsidl label.

@bzbarsky
Copy link
Collaborator

@domenic
Copy link
Member

domenic commented Apr 30, 2020

Let's close this until we have a concrete consumer.

@domenic domenic closed this Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants