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

Guidance requested on specifying an unusual pattern from Streams in Web IDL #819

Closed
domenic opened this issue Nov 3, 2019 · 7 comments
Closed

Comments

@domenic
Copy link
Member

domenic commented Nov 3, 2019

Streams would like to move to Web IDL. One particular pattern we've employed is unusual and I'd like help from the Web IDL community in figuring out what we should do about it.

In particular, the pattern in question is the queuing strategy classes, e.g. CountQueuingStrategy. These are intended to be basically convenience factories for objects of the form { highWaterMark: number, size: function }, used like new ReadableStream(..., new CountQueuingStrategy({ highWaterMark: 1 })).

This manifests currently as them having a data property highWaterMark, and a size() semi-method. The highWaterMark can transition fairly easily to a Web IDL readonly attribute/JS getter-on-the-prototype. The tricky one is our size() semi-method. I say semi-method, because it intentionally does not use its this value.

This no-this-value property is something we want to preserve, if possible. The streams machinery currently does not call these functions with a this value. It could be changed to, but as @ricea says,

I don't want to give up on calling size as a plain function. The principle that it is a pure function is important to the mental model of streams. Making it a method gives the implication that it can be stateful.

Stepping back a bit, I think it was kind of a mistake to make these queuing strategies into classes. They're really more like "factory functions for dictionaries". Moving to a Web IDL framework makes this more apparent. Anyway, how do we move forward? We have a few options (some previously discussed in whatwg/streams#1005) for specifying this within the framework of Web IDL:

  • Behavior preserving:

    1. Use "custom bindings", i.e. some prose which installs such an unusual method on the prototype manually, outside of the Web IDL system. Or add some kind of normative monkeypatch in the Streams spec saying "ignore these parts of the Web IDL algorithm". Fairly icky, but at least nobody will be tempted to copy us. (In practice I would expect implementations to use the following strategy under the hood, even if we specced this one.)

    2. Introduce an extended attribute to Web IDL, e.g. [NoThis], which removes the brand check from methods. Straightforward, but people might abuse this. Maybe we could call it [LegacyNoThis]?

  • Behavior changing:

    1. Specify size as a readonly attribute Function size; which returns the same function on all class instances. This changes the prototype property from a data property to an accessor, and it makes const size = CountQueuingStrategy.prototype.size throw. Neither of these consequences are too bad.

    2. Introduce an extended attribute to Web IDL, e.g. [LegacyAllowNew], which allows methods to be called even with new, and not throw. Then we could do something like

      dictionary CountQueuingStrategyInput {
        any highWaterMark;
      };
      dictionary CountQueuingStrategyResult {
        Function size;
        any highWaterMark;
      };
      partial interface mixin WindowOrWorkerGlobalScope {
        [LegacyAllowNew] CountQueuingStrategyResult CountQueuingStrategy(optional CountQueuingStrategyInput input);
      }

      Here we are assuming that it is not going to be sufficiently web-compatible to start throwing on new CountQueuingStrategy(), since currently you can only call it with new.

      This would change CountQueuingStrategy.prototype from its own object into just Function.prototype. And it would introduce another legacy extended attribute. But it has the nice advantage of actually fitting the mental model.

    3. Start treating size as a method, including brand checks, despite @ricea's misgivings. I want to include this here for completeness in case the Web IDL community does not agree with us on the mental model.

@bzbarsky
Copy link
Collaborator

Fundamentally, Web IDL already has a concept of functions that do not do a brand-check on this: static operations.

The difference is that in this cases we want to have a static operation on the prototype, not on the constructor, right?

I don't think there's a fundamental problem with this in general, apart from the added burden on spec authors of now trying to decide whether their statics should live on the prototype or the constructor, right? That said, is there a reason this specific property should live on the prototype, not the constructor, if it does not operate on instances?

In terms of how to accomplish this, syntax-wise, simplest is probably adding something to skip the brand check to non-static operations.... That's assuming we really do want to do this.

@domenic
Copy link
Member Author

domenic commented Mar 13, 2020

That said, is there a reason this specific property should live on the prototype, not the constructor, if it does not operate on instances?

I think it's a mistake; as I said in the OP, these should have been factory functions for a { Function size, unsigned long long highWaterMark } dictionaries, instead of a class.

Your reply seems to be working in the "behavior preserving" space from the OP, but thoughts are also welcome pushing us toward the "behavior changing" space.

@bzbarsky
Copy link
Collaborator

I'm not sure I completely understand the implications of the Function case. Would the browser be implementing this shared cached function? What would its lifetime and sharing characteristics be tied to?

@domenic
Copy link
Member Author

domenic commented Mar 13, 2020

Yeah, it would be in the spec/implementation manually. I guess it would be per-global. That sounds pretty icky, when I write it out...

@bzbarsky
Copy link
Collaborator

A little icky, but not too bad. Gives an obvious place to store it and a sane lifetime...

@ricea
Copy link
Contributor

ricea commented Mar 16, 2020

In Blink a new function is created every time the attribute is read (Blink has already switched to WebIDL for Streams). This is sub-optimal. Per-global would be better, provided I can create it lazily.

@domenic
Copy link
Member Author

domenic commented Jul 27, 2020

Closing this, as Streams decided to go with the "a little icky, but not too bad" version. See e.g. https://streams.spec.whatwg.org/#blqs-internal-slots and https://streams.spec.whatwg.org/#blqs-size

@domenic domenic closed this as completed Jul 27, 2020
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

3 participants