search

ipfs / js-ipfs

IPFS implementation in JavaScript
https://js.ipfs.tech
Other
7.44k stars 1.25k forks source link

Replace node Buffers with Uint8Arrays #3220

Closed achingbrain closed 3 years ago

achingbrain commented 3 years ago

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

Weedshaker commented 3 years ago

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 commented 3 years ago

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

jacobheun commented 3 years ago

Some notes on transitioning:

achingbrain commented 3 years ago

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 commented 3 years ago

BufferList has “shallowSlice” which is roughly what subarray does, just returning a new BufferList instead of Uint8array. But I would imagine that what you often want from BufferList is still .slice() because you probably need a complete binary value.

-Mikeal

From: Alex Potsides notifications@github.com Sent: Tuesday, August 11, 2020 7:53:04 AM To: ipfs/js-ipfs js-ipfs@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [ipfs/js-ipfs] Replace node Buffers with Uint8Arrays (#3220)

It's worth noting that .subarray cannot be used with BufferListhttps://www.npmjs.com/package/bls, which occasionally complicates matters.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/ipfs/js-ipfs/issues/3220#issuecomment-671994548, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAAAEQ6T3ERS552OHDBPVPLSAFLNBANCNFSM4P2DZHJQ.

wemeetagain commented 3 years ago

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

achingbrain commented 3 years ago

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.

achingbrain commented 3 years ago

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

HexaField commented 3 years ago

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

achingbrain commented 3 years ago

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