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

Backwards compatibility of ipfs.add(urlSource(url)) #3195

Closed
ipfs/js-ipfs-utils
#53
@Gozala

Description

@Gozala

This came up in #3081 (comment) and me and @achingbrain agreed to continue discussion on the separate channel (this issue)

Problem

  1. ipfs.add is supposed to take a single file input (right now it will happily accept many, but that is going to change in the future).
  2. ursSource returns multiple files (see: https://github.com/ipfs/js-ipfs-utils/blob/master/src/files/url-source.js#L5)

Options to address this:

  1. Make ipfs.add support multiple files.
  2. Make ursSource return singe FileObject.
  3. Embrace native data types (E.g. DOM Response object)

My personal opinions are as follows

  • I really don't like the option 1. It would prevent us from taking advantage of add / addAll separation to reduce complexity.
  • Option 2. seems like a good compromise
    • Looking into urlSource implementation, there is no reason it for it to return AsyncIterable<FileObject> instead it could return { path: string, * content() { yield * (await new Http().get(url, options)).iterator() } }.
    • What's bad about urlSource is that it will cause buffering in browser because of streaming.
      • This could probably addressed by introducing another code path for browser which would return { path: sting, content: AsyncIterable<Blob> }
  • Option 3. Makes most sense to me, but it's a larger breaking change
    • urlSource essentially returns an async task (not the result) that add will run and consume results
      • This seems wrong to me add is not a scheduler for running tasks, it should take results instead.
    • Standard Response represents result of fetching remote data.
    • Conveniently it allows consumer to choose betwen blob and stream consumption strategies.
    • However it would require await fetch(url) on user end, but that makes sense (add isn't a task scheduler).
      • If we add should take on a task scheduler role, then it could accept standard Request instances.

With all that said I think best compromise would be to make ursSource return FileObject and I can take on that as a followup to #3081 that would enable tests that use urlSource now.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions