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

[wip] First pass at adding internal slots #495

Closed
wants to merge 1 commit into from

Conversation

tobie
Copy link
Collaborator

@tobie tobie commented Dec 10, 2017

This is a rough first pass at adding internal slots and a "this" keyword to WebIDL.

I originally intended to write a design doc, but thought it would be easier to work directly in the spec adding comments as issues where needed.

I think this captures the key parts of the discussions that happened in #258 and on https://www.w3.org/Bugs/Public/show_bug.cgi?id=27354.

High-level comments more than welcomed.

TL;DR:

  1. There's now a this keyword in WebIDL
  2. There are now internal slots (e.g. foo.[[slot]]).
  3. Each attribute gets its own internal slot.
  4. By default, attribute getters read from their slot and setters write to it. But you can completely override this in prose.
  5. Additionally, You can define your own internal slots on top of those. It's still TBD whether those will appear in the WebIDL directly or not.
  6. Unsure whether slots should have default (initial) values (that would come from attribute default value and/or from dedicated syntax for custom internal slots).

Preview | Diff

@tabatkins
Copy link
Contributor

  1. Please add the actual syntax to the interface. Having them be separate is annoying both to define and to read; you have to go search for the internal slots in the surrounding prose. It's also makes the pattern far less obvious for neophyte spec authors. Finally, it means that Bikeshed can handle all the defining stuff for you, which makes it much easier for all spec authors, regardless of experience level.

  2. I don't think we should include an initial value. First, these are just attributes that happen to not have a public getter/setter, and attributes don't have initial values defined in the IDL; second, you can only express IDL primitives in that syntax, and most slots I've seen are not simple primitives; they're often Infra structures and/or objects created with particular meaning that can't be expressed in WebIDL. Since most slots will have to have their value defined in prose, it's more consistent to require all of them to do so.

  3. If my (2) is accepted, then we don't need to define the initial value of a slot; we can just require that specifications define that in prose.

Otherwise +1 to this, this fulfills my desires!

@tobie
Copy link
Collaborator Author

tobie commented Dec 11, 2017

Please add the actual syntax to the interface.

So we agree that this would only be for custom slots, right? i.e. the following snippet would have both a [[secret]] and a [[name]] slot, right?

interface Person {
  [[secret]];
  attribute DOMString name;
};

I don't think we should include an initial value. First, these are just attributes that happen to not have a public getter/setter, and attributes don't have initial values defined in the IDL;

So I'm not really sure whether initial values are useful or not. But if we think so, then the idea would be to add them to both slots and attributes:

interface Person {
  [[secret]] = "45e8173d40ddff8dcf81697326e094bcf8b92920";
  attribute DOMString name = "John Doe";
};

this.[[secret]] would have the value "45e8173d40ddff8dcf81697326e094bcf8b92920" and this.[[name]] would hold the string "John Doe".

@tabatkins
Copy link
Contributor

So we agree that this would only be for custom slots, right? i.e. the following snippet would have both a [[secret]] and a [[name]] slot, right?

Yes, exactly. I just want all the information that's meant to be attached to an object to be together in its IDL definition; it makes the shape of the object much more obvious.

So I'm not really sure whether initial values are useful or not. But if we think so, then the idea would be to add them to both slots and attributes:

Okay, but I don't think either should have it. As I said before, the only things you can set in WebIDL syntax are IDL primitives, and it's not all that common for the value of slots (custom or attr-based) to be primitives.

@annevk
Copy link
Member

annevk commented Dec 13, 2017

  1. We need a way to disable slot generation, e.g., for innerHTML. It's probably not acceptable that it would generate an unused slot. It's unclean and implementations would end up adding proprietary syntax for it. I suggested [NoSlot] in the original bug.

  2. When I look in DOM for "has an associated" it seems default values of

    • null
    • the empty string
    • string constants, e.g., "no-quirks"
    • false (for initially unset flags we'd convert to booleans)

    would see quite some use. There's also a couple cases where it cannot be used though, such as URLs, empty sets, empty lists, and MIME types. It might be confusing to have some in IDL and some in prose. (Maybe if we ever have a more unified type system it'd be more doable.)

  3. I agree with having all slots in IDL interfaces. If we also want concept-level slots we should add those to Infra, but I think we can continue to use simple prose for those.

  4. As for "this", an idea I had at some point is that IDL effectively generates the same of the algorithm that the specification needs to define. Say ElementNamespaceURIGetter. And then it invokes that algorithm name it generated with a reference to the object itself as the first argument and the remaining arguments as subsequent arguments. I still think that'd be great as it fully closes the formalization gap, but I realize it's quite a bit of work to get done.

    Another benefit of this is that we clearly separate the internal and public endpoints and now by default have an internal algorithm to call for each algorithm we may want to reuse somewhere.

    Probably the most pragmatic step is to just define a magic keyword as you did, unless there's more enthusiasm for my idea.

@tobie
Copy link
Collaborator Author

tobie commented Dec 13, 2017

Thanks, @annevk!

  1. We need a way to disable slot generation, e.g., for innerHTML. It's probably not acceptable that it would generate an unused slot. It's unclean and implementations would end up adding proprietary syntax for it. I suggested [NoSlot] in the original bug.

I feel like that's an implementation detail, but I'm not hostile to adding a dedicated extended attribute if there's consensus for that.

  1. When I look in DOM for "has an associated" it seems default values of

    • null
    • the empty string
    • string constants, e.g., "no-quirks"
    • false (for initially unset flags we'd convert to booleans)

    would see quite some use. There's also a couple cases where it cannot be used though, such as URLs, empty sets, empty lists, and MIME types. It might be confusing to have some in IDL and some in prose.

I agree on all counts. This is also something we can add on at a later stage.

(Maybe if we ever have a more unified type system it'd be more doable.)

Not sure what you mean by that.

  1. I agree with having all slots in IDL interfaces. If we also want concept-level slots we should add those to Infra, but I think we can continue to use simple prose for those.

Agreed.

  1. As for "this", an idea I had at some point is that IDL effectively generates the same of the algorithm that the specification needs to define. Say ElementNamespaceURIGetter. And then it invokes that algorithm name it generated with a reference to the object itself as the first argument and the remaining arguments as subsequent arguments. I still think that'd be great as it fully closes the formalization gap, but I realize it's quite a bit of work to get done.

    Another benefit of this is that we clearly separate the internal and public endpoints and now by default have an internal algorithm to call for each algorithm we may want to reuse somewhere.

    Probably the most pragmatic step is to just define a magic keyword as you did, unless there's more enthusiasm for my idea.

That's interesting. I need to let it sink in a bit. Maybe "this" or your alternative proposition here can be added separately if we're not sure of them yet.

@domenic
Copy link
Member

domenic commented Dec 13, 2017

Especially as this grows extra complexity such as IDL syntax, [NoSlot], default values, etc. I become increasingly convinced this does not pay for itself. Introducing this is good. Beyond that I think Web IDL's only role for slots should be to say "here's one way you could declare internal data", not to add syntax.

What would change my mind is implementations stating that they would use this information in the IDL to help generate good code. But I find this quite unlikely given how current code generation works.

In short, I don't think this is an improvement over what we're doing today.

@bzbarsky
Copy link
Collaborator

and implementations would end up adding proprietary syntax for it.

I'm not quite sure, as an implementor to what extent I would end up generating slots based on IDL information anyway.

If I'm generating the entire implementation (e.g. this is what Gecko does with a number of Event subclasses), then it makes sense to generate internal slots, of course. Right now we do this precisely by generating slots for all the attributes. This only works for cases in which there is nothing interesting going on with the slots other than values being stored in the constructor and then read and maybe set via attributes.

But there are lots of cases in which whether there's a slot is an implementation decision. Let me give one "simple" example. Element has tagName and localName attributes. Per spec, localName just does a slot read, while tagName runs an algorithm including getting the "qualified name", which it itself an algorithm that reads some slots, includingf the "local name" slot.

In Gecko, all three of "local name", "qualified name", and "tagName" are computed ahead of time and stored on the element. But they're not really stored in slots on the element itself, in implementation terms. What's stored in a slot is a pointer to a shared data structure (shared across all elements with the same namespace/prefix/localName in the element's node document) that stores all these bits. That means we compute them once on a per-document basis (instead of the spec's per-get computation, getters are fast, etc. This all works because all of this state is immutable.

But nothing requires things to work this way. A different implementation could make different tradeoffs here in terms of what's stored in slots and what's computed and what's shared and so forth.

@jyasskin
Copy link
Member

2¢: I really like the idea of listing all the slots in the interface {...} definition. For slots that match 1-1 with an attribute that reflects them, I'm happy with any of the 3+ options for listing the name only once. I'd be somewhat unhappy having to list both the slot and the attribute that reflects it.

I'd like specs to define the type of all of their slots. When the slot is reflected by an attribute, it makes sense to use a WebIDL type for the slot. But lots of slots have types that aren't representable by WebIDL. For example, BluetoothDevice.[[representedDevice]] holds the platform's idea of a Bluetooth device. For these, prose should be required to specify the type of the slot, and maybe a "SeeBelow" keyword in the IDL would be helpful in reminding spec authors to do that.

@tobie
Copy link
Collaborator Author

tobie commented Dec 18, 2017

So just to clarify, the 3 options @jyasskin refers to above were discussed on irc and are:

  1. The above proposal:

    interface I {
      [[aSlot]]; // custom slot
      readonly attribute DOMString foo; // has a [[foo]] slot.
      attribute DOMString innerHTML; // has a [[innerHTML]] slot, never uses it.
    };
    
  2. @domenic's option (opt-in):

    interface I {
      [[aSlot]]; // custom slot
      [Slot] readonly attribute DOMString foo;  // has a [[foo]] slot.
      attribute DOMString innerHTML; // has no slot.
    };
    
  3. Ruby-style:

    interface I {
      [[aSlot]]; // custom slot
      [[anotherSlot]]; // another custom slot used below
      [Reflect=anotherSlot] readonly attribute DOMString foo; // reads from [[anotherSlot]]
      attribute DOMString computedAttr; // has no slot.
    };
    ```
    
    

To which I'll add @annevk's option:

  1. @annevk's option (opt-out):
    interface I {
      [[aSlot]]; //custom slot
      readonly attribute DOMString foo; // has a [[foo]] slot.
      [NoSlot] attribute DOMString computedAttr; // has no slot (duh!)
    };
    

@tabatkins
Copy link
Contributor

I hate (3) for the same reason @jyasskin said. I prefer (1) or (4) (4 being just an optimization on 1), but I could live with (2) if necessary.

I'd like specs to define the type of all of their slots.

Yes, strongly agree, and having a "type" that means it's defined in prose instead sounds good.

@domenic
Copy link
Member

domenic commented Jan 12, 2018

The talk about types makes me further think these should not be in the IDL syntax, which has its own type system, but in a separate table, which can use the spec/infra type system.

@annevk
Copy link
Member

annevk commented Jan 13, 2018

I don't think IDL's type system should be different; it's a subset. When IDL says "USVString" that should convert a JavaScript string to a scalar value string (and vice versa when used for a return value). "ByteString" should convert a JavaScript string to a byte sequence (and a byte sequence to a JavaScript string for the return value).

I don't think it should be separate. I want the class/interface description to take care of these so it's clear they're allocated when the class/interface is (and IDL can say as much). Describing each object with two pieces of data would still allow for that, but it gets messier as you first have to do some kind of "find the other bits of data" step.

We should make it clear that implementations can have different strategies and use pointers to share slot data and such, but that's already the case for all algorithms. I don't think that takes away that specifications should strive for clarity and a single object description provides that.

@tabatkins
Copy link
Contributor

Ehhhhhh, I just really want this to be as easy as possible for people, so I can build it into Bikeshed automatically. We have too many people accidentally referring to public APIs in spec text, and requiring them to write up a totally separate thing that's non-obvious but looks like it should be an automatic obvious sort of thing (that is, a <dfn attribute for=SomeInterface>\[[bar]]</dfn> in their spec) just increases the friction and reduces the chance of them getting it right.

You're right, tho, about types - I dunno what I was thinking, since "I want to declare types in IDL" is exactly opposed to "I don't think we can generally put default values in IDL". Typing is nice, but it's not compatible with the current mix of IDL and Infra values.

@tabatkins
Copy link
Contributor

Can we prioritize this? The spec as written seems fine to me; maybe adding [NoSlot] is the last thing that needs to be added for a feature-complete v1. I'd like to make Bikeshed auto-define the slots for things to I can link to them; my current practice in Typed OM of referring the the object's {{Foo/bar}} private slot is confusing to @annevk, and I'd prefer to write the object's {{Foo/[[bar]]}} slot instead.

@littledan
Copy link
Collaborator

I've observed accidental exposure of public APIs in early draft specifications due to the lack of this feature as well.

What was the motivation for creating all of these implicit slots, and then needing a [NoSlot] extended attribute? How about we just make all these slots explicit?

As for type system, I like the idea of keeping these internal slots untyped (matches current usage). Could we make the ECMAScript undefined value the default (matching current usage), or would this be too big of a layering violation given how we don't have undefined in WebIDL yet?

Minor: ECMAScript switched to internal slots all starting with a capital letter. Now, lower case internal slot names look ugly to me. I'm fine with using lowercase if others prefer it, though.

@tabatkins
Copy link
Contributor

The reason to not require them all to be explicit is that literally almost every single attribute needs a slot. Almost every attribute is a getter/setter pair that manipulates the internal slot; the only exceptions are the rare "weird" attributes that map to more complicated stuff like .innerHTML. Requiring editors to write identical boilerplate for 95+% of attributes is absolutely backwards.

(And in the implicit-slot world, [NoSlot] isn't even a behavioral difference, just a hint to the implementor that the spec's algorithms will never manipulate the corresponding internal slot, so it can be elided from the internal representation entirely.)

@annevk
Copy link
Member

annevk commented Dec 12, 2018

I wanted attributes to be somewhat automatically backed by slots for these cases:

  • Defining new events. Currently we cheat for events and don't say that https://dom.spec.whatwg.org/#interface-customevent for instance also has a [[Details]] internal slot. I was hoping that making this defined could be done without any effort on specification editors.
  • Defining getters without prose. CustomEvent has a details getter, but all it does is returning [[Details]]. This could follow from IDL rather than requiring boilerplate. (This might also apply to setters that only do type coercion but I have a hard time coming up with an example.)
  • More speculative: when allocating a new platform object, attributes on that platform object backed by their own platform object could automatically have their slot filled with a platform object of that type created in a consistently-chosen Realm. (E.g., consider document.implementation.)

I don't really want to block on this though, having shared understanding and a consistent editorial policy around private state is much more important to me.

@domenic
Copy link
Member

domenic commented Dec 12, 2018

I don't think the percentage of slot-backed attribute is as high as 95+%. Especially if you take into account special setter logic that isn't just "set to the value", but also performs some state-checking or validation.

That said, it's probably over 50%. (We'd need to do a survey to be sure.)

My preferred way of threading this needle would be that slots should be explicit, but it should be easy to auto-generate getters (and optionally setters) from the slots. For example

interface CustomEvent {
  [[detail]];
  get detail <- [[detail]];

  // or (no separate slot declaration):

  any [[detail]] -> get detail;

  // or, using existing attribute syntax:
  [[detail]];
  readonly attribute any detail <- [[detail]];

  // or (no separate slot declaration):
  readonly attribute any detail <-> [[detail]];
}

(Lots of other permutations of these possible; these are intentionally not consistent to illustrate how there are lots of choices.)

My general feeling is also opposite @annevk's; I don't think there's any value in creating yet another convention for private state, and hoping people use it, without adding some code/spec-generation benefits at the same time, of the sort listed in his bullet points. Just relocating the same information from <dl>s or <table>s into IDL blocks does not seem worthwhile, and in fact seems like a bit of a layering violation, in that currently IDL only defines observable public facets of a class.

@littledan
Copy link
Collaborator

Another possible idea, similar to Domenic's above: slots are generally declared separately, but there is a special construct to declare a slot and a attribute for it together, e.g.:

interface CustomEvent {
  readonly attribute any [[detail]];
}

The idea here would be that the public attribute name is derived from the internal slot name, rather than the other way around. This would be somewhat analogous to certain possible JS decorator idioms (e.g., @reader #detail;).

Anyway, seems like this will take more discussion, and it'd be worth landing the part about this separately.

@tabatkins
Copy link
Contributor

I don't think the percentage of slot-backed attribute is as high as 95+%. Especially if you take into account special setter logic that isn't just "set to the value", but also performs some state-checking or validation.

Having to do extra stuff isn't relevant to my point. In all those cases, the definition of the slot would be absolutely identical boilerplate, endlessly repeated. Much better if Bikeshed just does it for you automatically, so can jump straight into referring to it.

The % of attributes which don't use anything like a slot at all, and instead are purely backed by getter/setter code, is very small. Just look at any random spec.

Note that I'm not implying any guarantees about what's in the slot. Just that most attributes store/retrieve something from private storage on the instance.

The % of attributes that do nothing but store/retrieve an IDL value from an internal slot is likely high enough to be worth automatically addressing, but that's, I think, a larger discussion.


As per earlier discussion, having most slots actually written in the IDL is... weird. As stated by others earlier, slots don't necessarily have the same types as attributes; they can take Infra types too.

I'd be okay with a very low-information way of indicating additional private slots (beyond what's used just to back corresponding attributes). For example, in https://drafts.csswg.org/css-font-loading/#fontface-interface, all of the attributes have trivial backing slots, just storing/retrieving an IDL value. In addition, the interface has [[Urls]] and [[Data]], which are internal slots used for other things, but not exposed via trivial accessors. Being able to write

interface FontFace {
  ...
  slot [[Urls]];
  slot [[Data]];
}

would be nice, as it (along with all the implicit attribute backing slots) tells you precisely what data will be associated with the object right there in the IDL, rather than requiring digesting all the associated prose.

@marcoscaceres
Copy link
Member

2c: it would be great if if slots have a type information associated with them.

@annevk
Copy link
Member

annevk commented Mar 25, 2019

@marcoscaceres could you elaborate? We discussed that upthread I think and the conclusion was that since we don't have a unified type system (yet) that would not really be possible to do.

@marcoscaceres
Copy link
Member

@marcoscaceres could you elaborate?

Sure, my experience has been that internal slots generally only hold booleans, strings, promises, dictionaries, or other IDL types. For example, generally speaking PaymentRequest's internal slots have these types:

list<string> [[serializedMethodData]] 
list<string> [[serializedModifierData]]
object [[details]]
PaymentOptions [[options]]
string [[state]]
boolean [[updating]]
promise [[acceptPromise]]
PaymentResponse [[response]]

Which are either IDL type or infra types. Ideally, I'd like to just declare them upfront in the IDL. So I guess part of the ask is for us to try to unify on the infra types as primitives.

Thus, would really like something like this:

interface PaymentRequest : EventTarget {
  slot list<string> [[serializedMethodData]]; 
  slot list<string> [[serializedModifierData]];
  slot object [[details]];
  slot PaymentOptions [[options]];
  slot string [[state]];
  slot boolean [[updating]];
  slot Promise [[acceptPromise]];
  slot PaymentResponse [[response]];
  /* good old IDL attributes, operations, etc. */
};

@jakearchibald
Copy link
Contributor

I really like this part of @domenic's suggestion:

interface Whatever {
  readonly attribute DOMString foo <- [[foo]];
}

Which would give [[foo]] a DOMString type.

It should be possible to do things like:

interface Whatever {
  [EnforceRange] unsigned long long [[bar]];
}

In this case [[bar]] is typed, and assigning a negative number to it will throw.

interface Whatever {
  [[thing]];
}

In this case, [[thing]] is untyped, and may be any IDL or infra type.

The distinction I want from slots (maybe this is stating the obvious) is that the values may not be changed/mutated from another thread.

@littledan
Copy link
Collaborator

@lukewagner's proposed evolution of WebAssembly/WebIDL bindings, presented in the June 2019 Wasm CG F2F, points a path towards a new, stronger specification-internal type system that we might reference here. However, I'm not sure if we can consider WebIDL types or infra structures as a formal enough type system to use here just yet.

@tabatkins
Copy link
Contributor

assigning a negative number to it will throw.

Throw how? Slots are only interacted with in spec prose, not by author code.

@jakearchibald
Copy link
Contributor

Spec prose can also throw. Eg step 2 of https://dom.spec.whatwg.org/#selectors.

@domenic
Copy link
Member

domenic commented Aug 2, 2021

Let's close this and keep the discussion open in #258, with the closed-but-outdated PR as a good example of one direction.

@domenic domenic closed this Aug 2, 2021
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.

9 participants