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
Description
This came up in #3081 (comment) and me and @achingbrain agreed to continue discussion on the separate channel (this issue)
Problem
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).ursSource
returns multiple files (see: https://github.com/ipfs/js-ipfs-utils/blob/master/src/files/url-source.js#L5)
Options to address this:
- Make
ipfs.add
support multiple files. - Make
ursSource
return singeFileObject
. - 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 returnAsyncIterable<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> }
- This could probably addressed by introducing another code path for browser which would return
- Looking into
- Option 3. Makes most sense to me, but it's a larger breaking change
urlSource
essentially returns an async task (not the result) thatadd
will run and consume results- This seems wrong to me
add
is not a scheduler for running tasks, it should take results instead.
- This seems wrong to me
- Standard
Response
represents result of fetching remote data. - Conveniently it allows consumer to choose betwen
blob
andstream
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 standardRequest
instances.
- If we
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.