helia icon indicating copy to clipboard operation
helia copied to clipboard

Helia UnixFs `addFile` does not respect `wrapWithDirectory`

Open 2color opened this issue 2 years ago • 3 comments

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 avatar Apr 19 '24 11:04 2color

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

2color avatar Apr 19 '24 11:04 2color

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 avatar Apr 23 '24 08:04 2color

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.

2color avatar Apr 23 '24 09:04 2color