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

Streams are old and legacy and should be refactored to v16 LTS #75

Open
CMCDragonkai opened this issue Aug 22, 2022 · 2 comments
Open
Labels
development Standard development r&d:polykey:core activity 2 Cross Platform Cryptography for JavaScript Platforms

Comments

@CMCDragonkai
Copy link
Member

Specification

The streams modules are still based on readable-stream 3.6.0, and also it ported from VFS but the implementation had gone through changes that haven't been fully verified. We should upgrade the readable-stream to 4.x.x https://www.npmjs.com/package/readable-stream which is cut from nodejs v18. Then refactor our streams abstraction, and also write tests to verify all the functionality.

Of particular note is the fact that our _destroy doesn't seem to look right. There's alot of callbacks and errors that are being threaded around.

Additional context

Tasks

  1. Upgrade to 4.x.x of readable-stream
  2. Review https://nodejs.org/api/stream.html#implementing-a-writable-stream and compare with our current implementation
  3. Consider reviewing the source of readable stream for the default implementation, as the main thing is the opening and closing of our EFS file descriptors
@CMCDragonkai CMCDragonkai added the development Standard development label Aug 22, 2022
@CMCDragonkai CMCDragonkai self-assigned this Jul 10, 2023
@CMCDragonkai CMCDragonkai added the r&d:polykey:core activity 2 Cross Platform Cryptography for JavaScript Platforms label Jul 10, 2023
@tegefaulkes
Copy link
Contributor

Should we convert to webstreams here?

@CMCDragonkai
Copy link
Member Author

We need to conserve the fs API. So we don't convert to web streams here. We just need to upgrade the fs stream readable stream library.

@CMCDragonkai CMCDragonkai removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development r&d:polykey:core activity 2 Cross Platform Cryptography for JavaScript Platforms
Development

No branches or pull requests

2 participants