Constructor specification is unclear

[jorendorff]

Is subclassing supposed to work?

class SpamStream extends ReadableStream {
    constructor() {
        let stream;
        super({
            pull(controller) {
                controller.enqueue("spam");
                stream.spamCount++;
            }
        });
        this.spamCount = 0;
        stream = this;
    }
    hasSpammed() { return this.spamCount > 0; }
}

new SpamStream().hasSpammed()

The standard currently says:

We prefix section headings with new to indicate they are defining constructors; when doing so, we assume that NewTarget will be checked before the algorithm starts.

What it doesn't say is how this gets initialized. Mozilla's current implementation ignores NewTarget and always creates an object whose [[Prototype]] is the original value of ReadableStream.prototype. So the above code would fail; hasSpammed would not be on that object's prototype chain.

I think we want something like

1. If NewTarget is undefined, throw a TypeError exception.
2. Let this be ? OrdinaryCreateFromConstructor(NewTarget, the name of the per-realm intrinsic object that is this class's original prototype, the List of this class's internal slots).

(Technically, this doesn't quite work, because OrdinaryCreateFromConstructor step 1 effectively asserts that it is being called from inside ECMA-262 and not from other standards. But I think it's an improvement anyway.)

[ricea]

I think developers would find it surprising if subclassing didn't work. My interpretation is that

This specification uses algorithm conventions very similar to those of [ECMASCRIPT], whose rules should be used to interpret it (apart from the exceptions enumerated below).

means that subclassing should work as it does for other constructors defined in ECMASCRIPT.

@domenic, is that what you intended?

[jorendorff]

I figured that sentence was referring to the Algorithm Conventions section of the ECMAScript spec. Those are all notational conventions—super low-level stuff.

Higher-level patterns, like conventions for how builtins should support subclassing, typically aren't mentioned in that spec, even informally. You have to read between the lines. (It is a slightly aggravating standard, in some respects. Aren't they all.)

[domenic]

Yeah, this is definitely not explicit (and not tested). We should probably for now add a new line to https://streams.spec.whatwg.org/#conventions, plus some tests.

Of note, #963 would actually make this worse, because https://heycam.github.io/webidl/#interface-object does not do the right thing yet. That's whatwg/webidl#533

[ricea]

If you override the constructor, but don't call super(), should IsReadableStream() still return true?

[domenic]

Nope, calling super() is essential. (This is how it works other ES and Web IDL stuff, apart from when ES stuff is generic.)

[ricea]

@yutakahirano added a test for ReadableStream in https://chromium-review.googlesource.com/c/chromium/src/+/1347643. I've got tests for TransformStream and WritableStream ready, but I'm waiting for web-platform-tests/wpt#14172 to land first.

[MattiasBuelens]

@ricea web-platform-tests/wpt#14172 finally went green, Firefox stability tests took a long time. Feel free to merge it, then you can rebase your new tests. 🙂

[ricea]

Subclassing tests for WritableStream and TransformStream landing soon at web-platform-tests/wpt#14228.

[MattiasBuelens]

@ricea Are you planning to update the ReadableStream is extendable test to align more with the new tests from web-platform-tests/wpt#14228? I think it'd be nicer if all three classes had similar tests.

[ricea]

@ricea Are you planning to update the ReadableStream is extendable test to align more with the new tests from web-platform-tests/wpt#14228? I think it'd be nicer if all three classes had similar tests.

Done: web-platform-tests/wpt#14235.

[Ms2ger]

WebIDL should now do the same thing; could people check if Streams matches IDL's behaviour?

[ricea]

WebIDL should now do the same thing; could people check if Streams matches IDL's behaviour?

Sorry, I didn't fully understand your request. Could you provide a link to the behaviour you want checked?

[Ms2ger]

Probably any section that starts with "new", but it seems like the creation of these objects is not defined at all, so that makes the request: define how your constructors work (in particular wrt NewTarget and subclassing).