Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Replace node Buffers with Uint8Arrays #3220

Closed
64 tasks done
achingbrain opened this issue Aug 10, 2020 · 10 comments
Closed
64 tasks done

Replace node Buffers with Uint8Arrays #3220

achingbrain opened this issue Aug 10, 2020 · 10 comments
Labels
awesome endeavour env:browser

Comments

@achingbrain
Copy link
Member

achingbrain commented Aug 10, 2020 •
edited
Loading

In order to treat browsers as first-class citizens, we should not use modules from node core unless we can absolutely guarantee that the code we are writing will not run in a browser.

The next generation of multiformats & IPLD modules will also be Uint8Arrays all the way down so embracing more modern formats will mean the upgrade path easier to follow when they are ready for use.

Modern JavaScript runtimes have TypedArrays such as Uint8Array backed by ArrayBuffers, as well as DataViews that wrap an ArrayBuffer and allow you to do bit-twiddly operations like writing little endian 32 bit floats at arbitrary offsets, etc.

Node's Buffer module pre-dates all of this but since node v3 it extends the Uint8Array class, so we should be able to treat Buffers as Uint8Arrays internally in our stack, though some dependencies may require Buffers as input until they too can be refactored.

PRs to modules

IPFS

IPLD

libp2p

Multiformats

Other
@ipfs ipfs deleted a comment from welcome bot Aug 10, 2020
@Weedshaker
Copy link

We could also adjust the examples. Eg.: https://github.com/ipfs/js-ipfs/tree/master/examples/ipfs-101

From
console.log('Added file contents:', Buffer.concat(chunks).toString())

To
console.log('Added file contents:', chunks.toString())

or at least make a note about the usage within Node/Browser.

@achingbrain
Copy link
Member Author

achingbrain commented Aug 11, 2020 •
edited
Loading

@Weedshaker when this effort bubbles up to a PR for this repo, yes, the documentation will be updated.

@jacobheun
Copy link
Contributor

Some notes on transitioning:

  • If Buffer.allocUnsafe is being used it should be retained for Node.js. There is no alternative for Uint8Array and there is a significant performance drop switching to Uint8Array only. Hot paths in Node.js should continue to use Buffer.allocUnsafe.
  • Buffer.slice(...) should become Uint8Array.subarray(...). Uint8Array's slice does a clone instead of referencing the underlying memory, which Buffer's slice and subarray does. subarray can be used for both Buffer and Uint8Array.

@achingbrain
Copy link
Member Author

achingbrain commented Aug 11, 2020 •
edited
Loading

It's worth noting that .subarray cannot be used with BufferLists, which occasionally complicates matters. .slice can, and it'll give you a no-copy view of an underlying buffer, unless the requested range falls over two internal buffers, in which case it's a copy operation.

@mikeal
Copy link
Contributor

mikeal commented Aug 11, 2020 via email

@wemeetagain
Copy link
Member

Is BufferList usage to be replaced with Uint8ArrayList (I don't think it currently exists)?

@achingbrain
Copy link
Member Author

The push here is really to have our ecosystem modules accept Uint8Arrays in place of Buffers and have them work without throwing errors.

For converting BufferList to Uint8ArrayList I think you'd have to do it for a module if it makes sense, but it may not - lots of our modules wrap other modules (crypto ones in particular) that only accept Buffers/BufferLists so from a practical point of view it may not be necessary or desirable until/if these wrapped modules can be refactored to accept Uint8Arrays.

@vasco-santos vasco-santos mentioned this issue Aug 27, 2020
Closed
19 tasks
@achingbrain
Copy link
Member Author

This shipped in js-ipfs@0.50.0! We did it!

This was referenced Sep 24, 2020
Merged
Closed
@HexaField
Copy link

https://github.com/ipfs/js-datastore-idb seems to have been missed

@achingbrain
Copy link
Member Author

@HexaField until ipfs/js-stores#198 is resolved, that module shouldn't be used anywhere in the stack, instead we should be using datastore-level - are you seeing problems?

matheus23 added a commit to oddsdk/ts-odd that referenced this issue May 18, 2021
@matheus23
Fix cbor encode/decode errors on node
dbc3b3d 
Switch to using Uint8Arrays instead of node Buffers.
This is something the js-ipfs folks apparently have gone through, too:
ipfs/js-ipfs#3220
matheus23 added a commit to oddsdk/ts-odd that referenced this issue May 27, 2021
@matheus23
Complete the Jest DI with crypto (#234)
ef3f1c9 
* Use ipfs-core instead of ipfs for a lighter dependency

* Make yarn node --version be 15.x in nix-shell

* Implement the complete crypto DI for node

* Fix cbor encode/decode errors on node

Switch to using Uint8Arrays instead of node Buffers.
This is something the js-ipfs folks apparently have gone through, too:
ipfs/js-ipfs#3220

* ESLint fixes

* Fix eslint errors in index.ts

* Revert "Fix cbor encode/decode errors on node"

This reverts commit dbc3b3d.

* Fix type

* Test cbor en/decoding

* Implement helper for totally in-memory ipfs

* Add integration test & fix filesystem running in node

* Remove logging

* Use types added to the workaround repo

* Fix CBOR decoding errors of encrypted data in node

* Switch back to borc

* Update jest & puppeteer & base58-universal

* Add jest puppeteer types

* Make it possible to run multiple ipfs in-memory

If I didn't disable swarm addresses, then multiple instances of ipfs would request socket access and block each other.

* Actually switch back from cborg to borc

* Remove unneeded Buffer.from call

* Make jest properly exit after all tests

* wtfnode

* Revert "wtfnode"

This reverts commit 5cbfd03.

* Force jest to exit, even if ipfs holds resources

* Revert "Make jest properly exit after all tests"

This reverts commit eab6bc8.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
awesome endeavour env:browser
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@mikeal @lidel @jacobheun @achingbrain @wemeetagain @HexaField @Weedshaker