Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite extract to user duplexer2 #128

Merged
merged 2 commits into from
May 22, 2019
Merged

Rewrite extract to user duplexer2 #128

merged 2 commits into from
May 22, 2019

Conversation

ZJONSSON
Copy link
Owner

closes #98

Rewrite extract to user duplexer2 to separate the events of the inbound stream and outbound stream. This allows errors and finish events to be more controlled.

Add tests for broken zipfiles (see #98 (comment))

…nd stream and outbound stream

add tests for broken zipfiles
@ZJONSSON
Copy link
Owner Author

This will probably end up as a minor bump, not a patch, since the return object from Extract changes from Parser to duplex with Parser as the in-stream and Writable as the out-stream. The usable functions and events should still be exactly the same as before (i.e. events: close, finish, error and method .promise())

Unsure what to do with typescript definitions as "object types" are changed, even in a minor way
(see original PR DefinitelyTyped/DefinitelyTyped#22858 and current type definitions: https://github.com/DefinitelyTyped/DefinitelyTyped/tree/c7004516e97da850d0872df71a0c326fc5e0581a/types/unzipper)

Is it better to actually include the types for the specific version in the unzipper package (and add "types": "unzipper.d.ts" to package.json)? If so I would love a PR that would demonstrate the ideal implemenation. Or should the DefinatelyTyped spec be updated to always represent the latest version of a library? I'm not a typescript user, but want to avoid breaking things for the typescript world.

cc @s73obrien @sabrehagen @MRayermannMSFT

@sabrehagen
Copy link

sabrehagen commented May 21, 2019 via email

@ZJONSSON
Copy link
Owner Author

Thanks @sabrehagen - merging to master. Will publish a minor bump when crx lands

@ZJONSSON ZJONSSON merged commit f594c65 into master May 22, 2019
@ZJONSSON ZJONSSON deleted the broken-zip branch May 22, 2019 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Errors - silently fails upon extracting
2 participants