Allow input to be a Stream. #17
Conversation
index.js
Outdated
| const type = fileType(input); | ||
|
|
||
| return type && type.ext === 'zip' ? processZip(input) : Promise.resolve([]); | ||
| } else if (isStream(input)) { |
There was a problem hiding this comment.
Don't need else if here since we're returning inside the previous if statement.
There was a problem hiding this comment.
True, personally think that else if says "or" better, but I'll change it.
index.js
Outdated
| }); | ||
|
|
||
| return getStream.buffer(input.pipe(firstChunkIsZip)).then(processZip, error => { | ||
| if (error === notAZipError) { |
index.js
Outdated
| } | ||
|
|
||
| return pify(yauzl.fromBuffer)(buf, {lazyEntries: true}).then(extractFile); | ||
| return Promise.reject(new TypeError(`Expected a Buffer or Stream, got ${typeof input}`)); |
There was a problem hiding this comment.
Don't move this to the bottom. I want to handle exceptions as early as possible.
There was a problem hiding this comment.
How is it different? Moving it to the top requires if (!isStream(input) && !Buffer.isBuffer(input)) before it. Those two expressions (besides negation) are already directly in front of this line.
There was a problem hiding this comment.
@kevva Still eagerly awaiting your response.
There was a problem hiding this comment.
Implemented this suggestion despite not really understanding the reasoning.
| const stream = fs.createReadStream(__filename); | ||
| const files = await m()(stream); | ||
|
|
||
| t.is(files.length, 0); |
There was a problem hiding this comment.
Should probably check the readable state of the stream here, do you know what is appropriate? I am thinking stream._readableState.flowing === false and stream._readableState.ended === false.
|
@kevva did you go on vacation? Would love to have some guidance on this if you are available. |
|
@kevva Would love to get this PR ready for merge while the information is still fresh in my mind. Also, it might be worth separating the "streaming file-type" part since something similar will likely need done for the |
|
Closing because questions have gone unanswered. |
|
@CodeMan99, I don't see the point in allowing a stream as input if we're going to buffer it anyway. That misses the whole point of streaming. Unless |
From conversation in issue #16.
The idea is that the first four bytes are read, then check file type. If not a zip, stop reading and return empty array. If it is a zip, buffer the stream and unzip as per normal.