ipfs / helia

An implementation of IPFS in JavaScript
https://helia.io
Other
812 stars 81 forks source link

Helia UnixFs `addFile` does not respect `wrapWithDirectory` #506

Open 2color opened 2 months ago

2color commented 2 months ago

Given the following code:

const helia = await createHelia()
const fs = unixfs(helia)

const cid = await fs.addFile({
  path: 'index.html',
  content: Uint8Array.from([4, 3, 2, 1])
}, {
  wrapWithDirectory: true
})

console.log(cid) // CID(bafkreihocdnev37gdi356hposn6kgiq27i5sgupz5i2o3o5xnfltyz4f64)

The fs.addFile returns a raw CID (bafkreihocdnev37gdi356hposn6kgiq27i5sgupz5i2o3o5xnfltyz4f64 and does not wrap the added file in a directory.

2color commented 2 months ago

It seems that this is also the case for fs.addBytes which doesn't respect wrapWithDirectory

2color commented 2 months ago

From @achingbrain

I’m not sure, I think maybe? The method is addFile, which it does, but if we don’t return a directory wrapper the filename is ignored. Since we’re accepting wrapWithDirectory, we should honour that and return the CID for the directory. Should we do this anyway if a path is supplied? What about a deeply nested path?

I guess the most obvious thing to do is to:

  • wrap the output in a directory if wrapWithDirectory is passed
  • wrap the output in a directory if a path is passed
  • deeply nest the output if a deeply nested path is passed

What if we pass a nested path and wrapWithDirectory? Do we ignore wrapWith.. since the returned CID would be the outermost directory anyway?

2color commented 2 months ago

Since we’re accepting wrapWithDirectory, we should honour that and return the CID for the directory.

👍

I guess the most obvious thing to do is to:

  • wrap the output in a directory if wrapWithDirectory is passed
  • wrap the output in a directory if a path is passed
  • deeply nest the output if a deeply nested path is passed

Makes sense to me

What if we pass a nested path and wrapWithDirectory? Do we ignore wrapWith.. since the returned CID would be the outermost directory anyway?

Also makes sense to me.