Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

RFC: Support ArrayBuffer as an alternative to Buffer input argument #404

Closed
lidel opened this issue Nov 29, 2018 · 7 comments
Closed

RFC: Support ArrayBuffer as an alternative to Buffer input argument #404

lidel opened this issue Nov 29, 2018 · 7 comments
Labels

Comments

@lidel
Copy link
Contributor

lidel commented Nov 29, 2018

Status

Right now we accept cid, multihash and data as Buffer in various places across entire JS API. It makes sense for Node.js and there was no better alternative for use in browsers at a time.

The landscape changed and we now have native vendor support for ArrayBuffer in web browsers:

The ArrayBuffer object is used to represent a generic, fixed-length raw binary data buffer. You cannot directly manipulate the contents of an ArrayBuffer; instead, you create one of the typed array objects or a DataView object which represents the buffer in a specific format, and use that to read and write the contents of the buffer.

Proposed API Improvement

Short and sweet:

  • Add support for ArrayBuffer as input in every API that accepts Buffer right now.

Why?

  • Low risk, High win:
    • This is backward-compatible change (Buffer support remains)
    • Should be a relatively small change (internally detect and convert ArrayBuffer to Buffer for now)
    • DRY: removes the need of tedious type conversion of various inputs
    • enables browser console experimentation without need for Buffer polyfil
    • Together with Async Iterators makes JS API feel modern an more attractive to developers

Refs

cc @alanshaw

@alanshaw
Copy link
Contributor

We've had multiple issues opened about this. People expect to be able to use them, are confused when they cannot and also, do not understand the difference between them and Buffer.

@vmx
Copy link
Contributor

vmx commented Nov 29, 2018

I'm in favour of it.

Side note: It's also something that came to my mind. I'm thinking about using ArrayBuffer for the new IPLD APIs. For input it's easy, you can support both. But for return values you have to make the call whether to return Buffer or ArrayBuffer.

@lidel
Copy link
Contributor Author

lidel commented Nov 29, 2018

@vmx I think the discussion about outputs is more complex and interesting, especially if you account for things that could land in the future eg. "zero-copy" ArrayBuffer pass from JS-land to WebAssembly-land etc.

If you think IPLD will need to tackle that decision soon, please create a separate issue for discussing ArrayBuffer in outputs. I feel we don't want to block input improvements by pending decision on outputs. :)

@achingbrain
Copy link
Collaborator

How about expanding what's accepted to array-like objects?

@lidel
Copy link
Contributor Author

lidel commented Dec 7, 2018

@achingbrain are you able to unpack that idea by providing an example? Do you mean objects with length and indexed items being Buffer or ArrayBuffer?

@achingbrain
Copy link
Collaborator

Sort of - I was thinking anything you can use in a for-of. We’d have to handle strings as a special case where node should interpret them as paths though, obvs.

@achingbrain
Copy link
Collaborator

This has been implemented - if there are more formats to support, please open a PR against js-IPFS with a failing test for the src/files/normalise-input function in ipfs-utils - https://github.com/ipfs/js-ipfs/blob/master/packages/ipfs-utils/test/files/normalise-input.spec.js

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants