MatrixAI / Polykey

Polykey Core Library
https://polykey.com
GNU General Public License v3.0
31 stars 4 forks source link

Replace `readable-stream` usage with Web Streams in the Vaults and Git Domains #523

Closed tegefaulkes closed 5 months ago

tegefaulkes commented 1 year ago

Specification

Some parts of the code are still using readable-stream. these need to be replaced with webstreams and the readable-stream dependency removed from the package.json.

These usages can be found at


[nix-shell:~/Projects/Polykey]$ ag "from 'readable"
src/vaults/VaultManager.ts
24:import { PassThrough } from 'readable-stream';

src/git/types.ts
1:import type { PassThrough } from 'readable-stream';

src/git/utils.ts
22:import { PassThrough } from 'readable-stream';

Additional context

Tasks

  1. ~[ ] 1. Replace any readable-stream usage with stream/web.~ - replacing with async generator
  2. [x] 2. Remove readable-stream as a dependency from the package.json.
CMCDragonkai commented 1 year ago

The readable-stream is also being used by EFS. That can't really be changed, it has to simulate node streams. However it's a really old readable stream. Might be better upgraded to match the latest stream behaviour. Not sure.

CMCDragonkai commented 1 year ago

I'm putting this under #298 because the usage of readable streams is relevant to git and EFS which is still using the old readable stream implementation in order to simulate how Node streams work, but Node streams have changed quite a bit since the initial import from VFS.

CMCDragonkai commented 11 months ago

Ok vaults domain should be using web streams, same with git. The git domain needs significant refactoring.

The EFS has to continue using "fs streams", it just requires an update to the fs stream backing library, and then testing to see that everything still works.

CMCDragonkai commented 11 months ago

Ok vaults domain should be using web streams, same with git. The git domain needs significant refactoring.

The EFS has to continue using "fs streams", it just requires an update to the fs stream backing library, and then testing to see that everything still works.

tegefaulkes commented 6 months ago

This might actually be addressed by the #709 PR, I'll double check and include this issue as fixed if so.

CMCDragonkai commented 6 months ago

Yes so if this is done, remove the dependency from package.json too. It will continue to be used by EFS until it can be refactored too.

tegefaulkes commented 6 months ago

I double checked and it's all gone. I've removed the dependency in #709 and made it fixed by that as well.