require "stream/web" produces warning

[zdm]

Hi,
On node v16 it produces experimental warning.

(node:18876) ExperimentalWarning: stream/web is an experimental featur

Could you please use polyfill until stream/web feature will get stable status?

Also instead of using try/catch for load stream/web you can check process.version and load stream/web if node version is >= 16.5.0.

[regseb]

I have the same warning with Node v16.8.0. The Web Streams API has been added in Node v16.5.0 but is experimental. I think you should use web-streams-polyfill until the API is stable (streams.cjs).

(node:23167) ExperimentalWarning: stream/web is an experimental feature. This feature could change at any time
    at emitExperimentalWarning (node:internal/util:227:11)
    at node:stream/web:7:1
    at NativeModule.compileForInternalLoader (node:internal/bootstrap/loaders:312:7)
    at NativeModule.compileForPublicLoader (node:internal/bootstrap/loaders:252:10)
    at loadNativeModule (node:internal/modules/cjs/helpers:41:9)
    at Function.Module._load (node:internal/modules/cjs/loader:804:15)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:94:18)
    at Object.<anonymous> (/home/regseb/testcase/node_modules/fetch-blob/streams.cjs:7:31)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)

[zdm]

stream/web api will be experimental for a year or more, so we are doomed to see this warning everywhere for a long time. ;-(

[jimmywarting]

I want to use the stream version developers will probably use cuz they are not interchangeable.
you can't pipe data from web-streams-polyfill to node's whatwg:stream

Also instead of using try/catch for load stream/web you can check process.version and load stream/web if node version is >= 16.5.0.

I also use this pkg on other non-nodejs places like Deno - Deno don't have stuff like blob's backed up by the filesystem. so i would prefer not to use version detection.

Deno for instances already have a global stream api... but i doubt this will be long lived, both Deno and Node's blob currently support async blobs and they have the potential to read data from the disc in a async manner, but non have a way of actually getting a blob from the disc yet.

[zdm]

Could you, please, move stream/web initialization code to the Blob.prototype.stream function?
Like this:

var initialized;

try {
    const { Blob } = require( "buffer" );

    if ( Blob && !Blob.prototype.stream ) {
        Blob.prototype.stream = function name ( params ) {
            if ( !initialized && !globalThis.ReadableStream ) {
                initialized = true;
                try {
                    Object.assign( globalThis, require( "stream/web" ) );
                }
                catch ( error ) {

                    // TODO: Remove when only supporting node >= 16.5.0
                    Object.assign( globalThis, require( "web-streams-polyfill/dist/ponyfill.es2018.js" ) );
                }
            }

            let position = 0;
            const blob = this;

            return new ReadableStream( {
                "type": "bytes",
                async pull ( ctrl ) {
                    const chunk = blob.slice( position, Math.min( blob.size, position + POOL_SIZE ) );
                    const buffer = await chunk.arrayBuffer();
                    position += buffer.byteLength;
                    ctrl.enqueue( new Uint8Array( buffer ) );

                    if ( position === blob.size ) {
                        ctrl.close();
                    }
                },
            } );
        };
    }
}
catch ( error ) {}

[jimmywarting]

It have been on my mind. we could do something like that.
But we should probably expose a function that can run the initialized part

the stream are needed here also:

fetch-blob/index.js

Lines 154 to 164 in 0e07457

	stream() {
	const it = toIterator(this.#parts, true);
	return new ReadableStream({
	type: 'bytes',
	async pull(ctrl) {
	const chunk = await it.next();
	chunk.done ? ctrl.close() : ctrl.enqueue(chunk.value);
	}
	})
	}

[zdm]

Yes, or export the function and call it in the stream() method.
Let's do this, because this warning is really annoying. ;-)

[jimmywarting]

(It is also possible to run NodeJS with the flag --no-warning)

This project is still quite Browser & Deno friendly and dose not really depend on any built in NodeJS stuff and I would also like to keep it that way.
Deno still lacks ways of creating Blob/Files backed up by the FileSystem. So i'm having thoughts of using this in Deno for another project i have.

So perhaps stream.cjs should expose a global function instead: globalThis._loadStreams() (b/c .cjs can't use esm export)
I have been thinking of using conditional import with top level await, but that requires node v14.14...

[zdm]

Disable warnings is a bad practice, warnings are useful, but not in our case.
Are you really using deno? I am unable to imagine, where it can be used and what for. Only as hobby. ;-)

[jimmywarting]

Are you really using deno? I am unable to imagine, where it can be used and what for. Only as hobby. ;-)

Yea, I use Deno for some own hoppy stuff. not so much for work.
tried convincing them to use Deno, but that would be a major rework. If i start with a new project/startup then i would try to use Deno instead if possible.

[zdm]

I think the correct way is to export from streams.cjs function like this:

/* c8 ignore start */
// 64 KiB (same size chrome slice theirs blob into Uint8array's)
const POOL_SIZE = 65536;

var ReadableStream;

module.exports = function getReadableStream () {
    if ( !ReadableStream ) {
        try {
            ReadableStream = require( "stream/web" ).ReadableStream;
        }
        catch ( error ) {

            // TODO: Remove when only supporting node >= 16.5.0
            ReadableStream = require( "web-streams-polyfill/dist/ponyfill.es2018.js" ).ReadableStream;
        }

        try {
            const { Blob } = require( "buffer" );

            if ( Blob && !Blob.prototype.stream ) {
                Blob.prototype.stream = function name ( params ) {
                    let position = 0;
                    const blob = this;

                    return new ReadableStream( {
                        "type": "bytes",
                        async pull ( ctrl ) {
                            const chunk = blob.slice( position, Math.min( blob.size, position + POOL_SIZE ) );
                            const buffer = await chunk.arrayBuffer();
                            position += buffer.byteLength;
                            ctrl.enqueue( new Uint8Array( buffer ) );

                            if ( position === blob.size ) {
                                ctrl.close();
                            }
                        },
                    } );
                };
            }
        }
        catch ( error ) {}
    }

    return ReadableStream;
};

And then use it where you need ReadableStream.
And you will not pollute globalThis.

[zdm]

What do you think?
If you want I can send pull req.

[zdm]

Hey, man, are you alive? Sorry for disturbing you, but let's close this issue and forget about it.

[jimmywarting]

Sry. I have been busy on other projects

Uhm, okey. Why?

[zdm]

I mean not just close, I can send PR with the fix, if you want.
Pls, look at my comment above.

[jimmywarting]

one smaller change could also be to do:

const orig = process.emitWarning
process.emitWarning = () => {}
Object.assign(globalThis, require('node:stream/web'))
process.emitWarning = orig

[zdm]

Yes, need to test. If this will work - it will be good.

…

On 17.09.2021 20:35, Jimmy Wärting wrote: one smaller change could also be to do: const orig = process.emitWarning process.emitWarning = () => {} Object.assign(globalThis, require('node:stream/web')) process.emitWarning = orig — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#114 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAH2MSGSXC4ACPTCMQ4Y55DUCN35VANCNFSM5DE25PZQ>.

[alasks332]

This is a bad idea to publish stable release which is depends on unstable or beta apis.

I think, that web/stream should be removed from the stable release and moved to the next major pre-release version.
If somebody requires blobs with the streams support he can install pre-release version separately.

Warning, that produced by this module requires attention, and must be removed.

[Arondight]

hi, will next version has a fix to this problem?

[jimmywarting]

I think, that web/stream should be removed...

with some degree i would be okey with that but i also need a prediction on what is best for the overall eco-system and guess what most ppl will use the most.

I advocate that ppl will prefer the latest version of web/stream first and formost, just b/c you don't need any polyfills and everyone will be using the same instance of stream 👈 most important

using the native streams by node also have some perf adv over a polyfill in a way that it's faster and can better be converted to from node/whatwg streams + that they are transferable via workers

The problem with web-streams-polyfill and any (native or polyfilled) ReadableStream is that they are not interchangeable with each other, this have been reported a numbers of times, and i have had problem with it myself in streamsaver.js and some other other streaming zip lib

• ReadableStream and File/Blob MattiasBuelens/web-streams-polyfill#97
• ReadableStream.prototype.pipeTo's first argument must be a WritableStream transcend-io/conflux#64
• signal with pre aborted  MattiasBuelens/web-streams-polyfill#75
• Improve brand checks MattiasBuelens/web-streams-polyfill#77

to summarize all the above problem with a code example you can't do this kind of things:

import {WritableStream} from 'web-streams-polyfill'
new window.Blob().stream().pipeTo(new WritableStream({ ... })) // throws
new window.Blob().stream().pipeTo(new window.WritableStream({ ... })) // works okey

this is going to throw an error: TypeError: Failed to execute 'pipeTo' on 'ReadableStream': parameter 1 is not of type 'WritableStream'

this will also be a problem if you use two different web-streams-polyfill versions
i depend on web-stream-polyfill v3.0.1 and you have v3.0.2 then we can't interchange streams

import {ReadableStream} from '[email protected]'
import {WritableStream} from '[email protected]'

new ReadableStream().pipeTo(new WritableStream({}))

this is going to throw the same kind of error: TypeError: Failed to execute 'pipeTo' on 'ReadableStream': parameter 1 is not of type 'WritableStream' cuz they are not the same instance of streams.

this is the hole underlying problem of why i do not want to bundle node-fetch into a 0 dependency cjs module. cuz we would then have one version and you would be using another.

I would be comfortable with only using web-streams-polyfill if this got fixed

• Investigate using native streams MattiasBuelens/web-streams-polyfill#20