sindresorhus / ky-universal

Use Ky in both Node.js and browsers
https://github.com/sindresorhus/ky
MIT License
670 stars 20 forks source link

body.getReader() method missing (node 14.x) #25

Closed jasonswearingen closed 1 year ago

jasonswearingen commented 4 years ago

There seems to be a problem using web-streams-polyfill on nodejs 14.x

I ran into the issue where, given some code like described in the documentation:

const ky = require('ky-universal');

(async () => {
    const {body} = await ky('https://httpbin.org/bytes/16');
    const {value} = await body.getReader().read();
    const result = new TextDecoder('utf-8').decode(value);

    // …
})();

I would get a runtime error:

TypeError: _a.getReader is not a function

at the 'body.getReader() call.

I read the docs to install web-streams-polyfill which I did (3.0.0) but still get the error.

I took stepped through the ky-universal code and see

//index.js
if (!global.ReadableStream) {
    try {
        global.ReadableStream = require('web-streams-polyfill/ponyfill/es2018');
    } catch (_) {}
}

This seems to be a problem because the require('web-streams-polyfill/ponyfill/es2018') returns an object, so the following fails:

//ky/umd.js
const supportsStreams = typeof globals.ReadableStream === 'function';

so.... clever me, I thought I would pre-shim the globals.ReadableStream to hot-patch the problem by adding this to my app code:

if ( !global.ReadableStream ) {
  global.ReadableStream = require( 'web-streams-polyfill/ponyfill/es2018' ).ReadableStream
}

but still, the body.getReader() function doesn't exist. Unfortunately the ky code is too fancy for me to follow, so here I am, filing this issue!

fyi, I also tried web-streams-polyfill@2.1.1 but same error.

sholladay commented 4 years ago

Indeed, I am able to reproduce this. For some reason, body ends up being a Node PassThrough stream, instead of a web compliant stream, even when web streams have been polyfilled. This is probably an issue with node-fetch rather than Ky or ky-universal. I will leave this open for tracking purposes, though.

AuHau commented 3 years ago

Well if you read node-fetch's Known differences from client documentation it is clearly specified there that they expose Node's Readable stream and not WHATWG's stream, so I don't think this is node-fetch problem but ky-universal...

sholladay commented 3 years ago

It doesn't really make sense for Ky to handle any other stream implementation besides web streams. But I'll agree with you in the sense that it's our fault for assuming that node-fetch would comply, or at least try to comply, with the specification. We're clearly going to have to switch to a better polyfill that properly implements the spec. See: https://github.com/sindresorhus/ky-universal/issues/34

sindresorhus commented 1 year ago

This package is no longer needed if you target Node.js 18 or later: https://github.com/sindresorhus/ky/releases/tag/v1.0.0