feat: replace into-stream to Readable.from
#290
Conversation
lib/utils.js
Outdated
| } | ||
|
|
||
| // Iterator | ||
| if (typeof payload === 'object' && typeof payload[Symbol.iterator] === 'function' && !isBuffer) { |
There was a problem hiding this comment.
Mayba add fast path if Buffer...
There was a problem hiding this comment.
It doesn't provides any goods but one more branch and === check.
| "mime-db": "^1.52.0", | ||
| "minipass": "^7.0.2", | ||
| "peek-stream": "^1.1.3", | ||
| "pump": "^3.0.0", | ||
| "pumpify": "^2.0.1" | ||
| "pumpify": "^2.0.1", | ||
| "readable-stream": "^4.5.2" |
There was a problem hiding this comment.
Not blocking but I just don't see the reason for adding readable-stream if the test suit is passing without it, I remember PRs that removed that package being merged as well
I get the stability argument, however with the same point of view one could say that any part of node's API can change in a breaking manner between major versions, no? Then why not install every single built-in module as a package?
But maybe readable-stream is special, so just asking to learn
There was a problem hiding this comment.
We can ensure that bugs in streams not solved in older node versions, like in node14, are not affecting our implementation.
|
@climba03003 I checked out this PR. New test case is failing at my local env. Not sure, why it is passed successfully in pipeline. |
Fixes #228
Closes #288
Checklist
npm run testandnpm run benchmarkand the Code of conduct