Conversation
ec7f248 to
b20d8ca
Compare
test/parallel/test-transform-by.js
Outdated
There was a problem hiding this comment.
I'm not sure what this test, tests for?
There was a problem hiding this comment.
in order to have an analogue to (chunk, enc, cb) => { } we add an encoding property for cases where transform streams behave differently depending on the encoding.
There was a problem hiding this comment.
I don't think this is taken into account in Readable.from and since we are doing objectMode I'm still unsure what the semantics here should be. I need to think about this a bit more unless someone can clarify?
941e354 to
30bc1ad
Compare
c4235a4 to
134db8b
Compare
134db8b to
62d6326
Compare
| Promise.all([ | ||
| transformBy(), | ||
| transformByFuncReturnsObjectWithSymbolAsyncIterator(), | ||
| transformByObjReturnedWSymbolAsyncIteratorWithNonPromiseReturningNext(), |
There was a problem hiding this comment.
@davidmarkclements: I don't think this test is needed. Returning a non promise is fine?
| else | ||
|
|
||
| if (!iterator || typeof iterator.next !== 'function') | ||
| throw new ERR_INVALID_ARG_TYPE('iterable', ['Iterable'], iterable); |
There was a problem hiding this comment.
@davidmarkclements Moved invalid arg check to Readable.from.
|
@davidmarkclements I've updated to include type checks |
|
@davidmarkclements: I don't understand the encoding test or implementation. Where else in streams related code do you see anything like this? Do you think you can explain it's purpose? |
5bf4f6e to
4bfffea
Compare
Tries to simplify nodejs#28501