ReadableStream internal state

[ronag]

While working implementing fetch in undici I've noticed a limitation on how compliant we can be towards the spec.

In order to "extract" a request body we need to have access to the streams [[disturbed]] state. However, this state is inaccessible through the public API.

Is there a way we could make this accessible behind a public symbol or something? Otherwise fetch can't be fully implemented outside of core as per my understanding of the specification.

[ronag]

@jasnell @benjamingr

[ronag]

@nodejs/whatwg-stream

[mcollina]

I'm +1 to expose this via a Symbol('nodejs.webstreams.disturbed').

@domenic wdyt?

[benjamingr]

I'll reiterate:

• We can't add properties or symbols to the stream, that would violate the spec. That point is very clear since it would hurt interop. Just like IE adding random properties to NodeList back when
• We are welcome to add statics or anything that is part of core and has access to internal state without changing the standard interface.

This is why we have a static getEventListeners rather than modifying EventTarget for example.

I vote a static disturbed (or better an object that exposes all of these) that works on both web and Node.js streams

[mcollina]

We can't add properties or symbols to the stream, that would violate the spec.

We'll end up adding something anyway as we would not have any other way to accessing this state. We might just decide to not expose this Symbol and have it in a not-enumberable property... however we'll need to have it there anyway.

[ronag]

We can't add properties or symbols to the stream, that would violate the spec.

We'll end up adding something anyway as we would not have any other way to accessing this state. We might just decide to not expose this Symbol and have it in a not-enumberable property... however we'll need to have it there anyway.

We already add symbols?

[benjamingr]

however we'll need to have it there anyway.

Adding things we don't consider our public API is risky but fine as long as it's implementation detail and we don't rely on it. If we can't even build without the internal API - that's a good indication we should expose that API (via a static method).

Since there is a simple, spec-compliant workaround here (a static method) I suggest we stick to that :)

(If we need an API like isDisturbed - contributing back to whatwg is also a good viable option, I am super busy with baby/new-job/new-house/family-health-stuff but technically that can be something I work on in my job, just not in the coming months)

[benjamingr]

whatwg/streams#1025 this looks positive

[domenic]

Adding non-spec statics is not spec-compliant; I'm not sure why you'd think they're any different from prototype properties.

[jasnell]

I would add separate methods that are not hung off the classes at all.

const { isDisturbed, isFinished } = require ('stream')

isDisturbed(readable)

isFinished(readable)

The challenge with these is that they will have to rely on internal state and not public API but that's fine.

[benjamingr]

@domenic I thought adding statics that are not part of the spec does not violate the spec while adding methods to the interface does. Did I misunderstand?

const s = getStream();
s.isDisturbed(); // not allowed, adds a method to the prototype
ReadableStream.isDisturbed(s); // not allowed, adds a method to ReadableStream
const { isDisturbed } = await import('nodejs/stream-utils');
isDisturbed(s); // this is fine