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

Defining AbstractModuleSource intrinsic inheritance #1322

Open
guybedford opened this issue Jun 16, 2023 · 13 comments
Open

Defining AbstractModuleSource intrinsic inheritance #1322

guybedford opened this issue Jun 16, 2023 · 13 comments

Comments

@guybedford
Copy link

guybedford commented Jun 16, 2023

The Source Phase Imports proposal requires that objects returned for the source phase must inherit from the new AbstractModuleSource intrinsic in their prototype chain.

In order for the WebAssembly ESM Integration to express this inheritance in WebIDL, once Stage 3 is obtained for the proposal, we need to determine how to represent this intrinsic inheritance for AbstractModuleSource.

Creating this as a new tracking issue for this interface type definitionquestion.

@bathos
Copy link
Contributor

bathos commented Jun 16, 2023

This seems akin to how DOMException interface objects inherit from %Error%. I don’t see how defining an AbstractModuleSource Web IDL interface would help with it — that would create a collision instead.

(i.e. if an “Error” interface had been defined, that wouldn’t mean its interface objects were %Error% — it would only mean its interface objects happen to have the same name. The globalThis.Error property would end up redefined to refer to these other Web IDL objects instead of the %Error% intrinsic during InitializeHostDefinedRealm, which is when define the global property references happens.)


I can imagine a new extended attribute for interfaces explaining both cases, e.g.

[Exposed=*, Serializable, IntrinsicHeritage="%Error%"]
interface DOMException { /*...*/ };

[LegacyNamespace=WebAssembly, Exposed=(Window,Worker,Worklet), IntrinsicHeritage="%AbstractModuleSource%"]
interface Module { /*...*/ };

I think that would also clean up how the existing case like this works, where it’s defined declaratively but isn’t actually integrated with the imperative steps for creating interface objects (a vestige from how the Web IDL spec used to be written?). It actually is in the imperative steps too I just realized — there’s a one-off call-out for DOMException in step 4 of “create an interface object”, which is correct ... but smells stinky, and it would be extra weird to add another unique case here rather than address it with a generic mechanism.

(The absent step for setting the [[Prototype]] of the constructor could likely be addressed at the same time.)

@guybedford
Copy link
Author

guybedford commented Jun 16, 2023

Good points. We probably should support both %AbstractModuleSource% and %AbstractModuleSource.prototype% chains here yes.

Perhaps that would mean two new attributes then?

[Exposed=*, Serializable, InstrinsicParent="%Error%", IntrinsicPrototypeParent="%Error.prototype%"]
interface DOMException { /*...*/ };

[LegacyNamespace=WebAssembly, Exposed=(Window,Worker,Worklet), IntrinsicParent="%AbstractModuleSource%", IntrinsicPrototypeParent="%AbstractModuleSource.prototype%"]
interface Module { /*...*/ };

It's quite a lot of machinery for a single case though that only possibly simplifies DOMException. Have there been other cases of this sort of thing?

While DOMException is an exception, we do have a similar thing for namespaces in the console spec at https://console.spec.whatwg.org/#console-namespace where it overrides the default prototype from the https://webidl.spec.whatwg.org/#namespace-object steps effectively with a comment. Perhaps we could use a similar approach.

@guybedford guybedford changed the title Defining AbstractModuleSource interface type Defining AbstractModuleSource intrinsic inheritance Jun 16, 2023
@annevk
Copy link
Member

annevk commented Jun 16, 2023

I wonder if @yuki3 or @saschanaz have thoughts about this.

@yuki3
Copy link

yuki3 commented Jun 16, 2023

My very personal preference is to define things systematically rather than defining things with ad-hoc rules (so, I tend to like the idea of [IntrinsicHeritage]). Having said that...

  1. Once it's explicitly supported (showing the existence of [IntrinsicHeritage] explicitly), I'm afraid that people would start abusing it more and more. E.g. "Oh, we can use this feature for our own unique purpose! (although not in an originally designed way)". I feel it's hard to prevent people from using existing features even when the features are explicitly marked as obsolete, deprecated, unrecommended, unsafe, etc.
  2. Given we have only two cases, I don't see the strong necessity of [IntrinsicHeritage].
  3. I'm fine with a special requirement of HTMLAllCollection's [[Call]] being not supported by an extended attribute.
  4. I'm fine with a special requirement of Location's "valueOf" property being not supported by an extended attribute.
  5. ...

I understand I'm talking about a sort of ...politics?... and not design of software, architecture, etc. Purely technically talking, [IntrinsicHeritage] looks good. I understand %X.prototype% is different from %X%.prototype, but [IntrinsicPrototypeParent] looks a bit overkilling/redundant (we don't want to support totally different inheritances for interface objects and prototype objects but it looks possible). Can we implicitly assume %X.prototype% inheritance, too?

In summary, my preference is: 1. special rule (no extended attribute) is enough (and good in total imho), 2. either of [IntrinsicHeritage] or [IntrinsicParent] + [IntrinsicPrototypeParent].

@petervanderbeken
Copy link

  1. Given how few cases there are at this point I also don't think we should add an extended attribute for this just yet.
  2. If this is done through an extended attribute there should only be one which then changes the prototype chain for both interface and interface object at the same time. For DOMException that would then fix Should DOMException.[[Prototype]] be %Error%? #1107 too.

@bathos
Copy link
Contributor

bathos commented Jun 16, 2023

Declarative comment / prose text “overrides” both have an unclear relationship to the imperative realm-initializing algorithms in Web IDL (when do the “overrides” happen?) and make it impossible for tooling that consumes Web IDL fragments to know about exceptional cases without hard-coding knowledge of them. Since I work on that kind of tooling, I’m biased to prefer an extended attribute solution.

In the past I think some EAs that aren’t quite “legacy” were given the Legacy prefix to discourage new “unbridled” usage?

[Edit: @yuki3, what would you think of making the permitted values for such an extended attribute an explicit enumeration defined by Web IDL? This might have the effect of keeping the Web IDL spec a bottleneck-by-design that prevents unaudited proliferation in specs that shouldn’t really be using this; extending a new intrinsic would always mean opening an issue here.]

If a non-IDL solution is used, then it would suggest to me that the existing imperative step for taking care of DOMException...

3.7.3. Interface prototype object, “create an interface object,” step 4

...would want to be removed? Or is the idea that the “create an interface object” algorithm would introduce a new step like this for WebAssembly.Module even though the interface is defined elsewhere?

BTW I think there is another example of this that’s also in WebAssembly? The NativeError-template Error classes. I’m unsure if they would benefit, but they do use use %Error%/%Error.prototype%.

@guybedford
Copy link
Author

guybedford commented Jun 16, 2023

Would it be possible to directly permit intrinsic inheritance for interfaces of the form:

interface DOMException : %Error% { ... }

interface Module : %AbstractModuleSource% { ... }

where this restricts intrinsic inheritance to be a specific case only - ie these must be well-defined intrinsic objects being subclassed, hopefully avoiding some kind of arbitrary object subclassing.

and then this intrinsic inheritance could itself be defined to include both the constructor and prototype inheritance chains, effectively in line with JS class inheritance.

Edit: I suppose it is not much different from a unified attribute, so this is more of a syntax question.

@saschanaz
Copy link
Member

Adding a new syntax is more expensive to the infra than the extended attribute option. Given that the latter is already being questioned as there are only few cases, I doubt we would go that path.

@bathos
Copy link
Contributor

bathos commented Jun 17, 2023

In addition to what @saschanaz said, new Web IDL syntax probably wouldn’t make sense here because this is really about the ES binding (which is what the extended attributes defined by Web IDL are for).

@guybedford
Copy link
Author

Okay, so it sounds like if anything it would be a single attribute with some handling for class-like behaviours. If we restricted the attribute to only applying to JS intrinsics though would that mitigate concerns of overuse? Or would such an approach still need to be better motivated?

@domenic
Copy link
Member

domenic commented Jun 17, 2023

I think we've gotten a pretty clear signal from multiple implementers that prose-based overrides for these rare cases is preferred?

@bathos
Copy link
Contributor

bathos commented Jun 17, 2023

@domenic It does look like that — two implementors stated a preference for prose, and I’m not an implementor (at least not in a sense that I would expect to count towards such a decision).

@yuki3
Copy link

yuki3 commented Jun 19, 2023

About when prose text "overrides" happens, I think it's nice to have something like "run an interface-specific configuration algorithm" in the imperative realm-initializing algorithms in Web IDL, and let each spec define "run an interface-specific configuration algorithm". This way has already been used in several places, right?

Just as a random idea; It might make sense and be helpful to have an extended attribute like [InterfaceSpecificConfiguration] in order to indicate that the interface has its own unique prose text "overrides". For example, we can use the extended attribute to cross-check whether we appropriately hard-code the behavior.

I'm skeptical about getting rid of hard-coded behaviors even if [IntrinsicHeritage] were introduced (e.g. Location's valueOf, HTMLAllCollection's [[Call]]). I don't think it will be helpful in case of Chromium. However, I don't have a strong objection especially when it will be very helpful for other people and/or projects. The concern about [IntrinsicParent] + [IntrinsicPrototypeParent] will remain, though.

EDIT: And it's welcome to restrict the coverage of [IntrinsicHeritage] in order to prevent abuse. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

8 participants
@guybedford @domenic @annevk @saschanaz @bathos @petervanderbeken @yuki3 and others