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

[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.

[lidel]

Unsure what would be the generic use case for urlSource in browser context ("paste URL here to import it to IPFS" page for re-hosting stuff?), but for what its worth IPFS Companion would most likely switch to urlSource for importing images/videos/pages via context menu on pages. For this specific use case, buffering should not be a problem, because the data is already in the browser cache.

I agree the best compromise for now is Option 2.

[Gozala]

Unsure what would be the generic use case for urlSource in browser context ("paste URL here to import it to IPFS" page for re-hosting stuff?)

Not sure what you mean by generic use case, but https://github.com/inkandswitch/xcrpt add-on we did crawl the page to archive it into IPFS. However as I pointed out in the description (quoting below)

• 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.

That makes urlSource(url) inadequate for that, because:

1. Some URLs are inaccessible

• Some resources can't be fetched (e.g. images) due to of CORS headers, even though they are in the cache.
• Some URLs may refer to not found resources (404).

2. To workaround some CSP issues you need to do some fetching in the add-on context as opposed to page context and pass things back and forth.

All this to suggest that ipfs.add / ipfs.addAll can't possibly recover from such errors, but user space code can. Which is why I find urlSource to be inadequate for most production use cases.

but for what its worth IPFS Companion would most likely switch to urlSource for importing images/videos/pages via context menu on pages. For this specific use case, buffering should not be a problem, because the data is already in the browser cache.

1. You will likely face problems outlined in the section above, which would make urlSource to be a poor choice.
2. If that is the case we should consider urlSource to use different code path in browser. Or else you'd fall of happy path as per Embracing web native FormData / File where possible #3029, because instead of passing blobs pointing to the in-memory resource you'd effectively be reading it via stream and then buffering it back into blob.