ipfs-inactive/js-ipfs-http-client

wrap-with-directory no working for latest few versions (39.0+ for sure)

obo20 opened this issue · 5 comments

obo20 commented

passing in the option "wrap-with-directory" doesn't appears to be working at all with newer versions of this client.

I've had to revert to 33.1.1 in order for this functionality to work correctly.

obo20 commented

Edit, it appears the required naming for this setting changed awhile back. I've tested further on 39.0.2 and things appear to be working

obo20 commented

I wanted to reopen this issue just so that the team was aware of the the backwards compatibility issue here since there doesn't appear to have been any documentation on the change(let me know if I'm wrong here).

Essentially the client used to use 'send-files-stream.js' to handle some options as seen here:

    const qs = options.qs || {}

    qs['cid-version'] = propOrProp(options, 'cid-version', 'cidVersion')
    qs['raw-leaves'] = propOrProp(options, 'raw-leaves', 'rawLeaves')
    qs['only-hash'] = propOrProp(options, 'only-hash', 'onlyHash')
    qs['wrap-with-directory'] = propOrProp(options, 'wrap-with-directory', 'wrapWithDirectory')
    qs.pin = propOrProp(options, 'pin')
    qs.preload = propOrProp(options, 'preload')
    qs.hash = propOrProp(options, 'hash', 'hashAlg')

In a recent version, it seems like things were switched over to using 'add/index.js' for the options as seen here:

    if (options.chunker) searchParams.set('chunker', options.chunker)
    if (options.cidVersion) searchParams.set('cid-version', options.cidVersion)
    if (options.cidBase) searchParams.set('cid-base', options.cidBase)
    if (options.enableShardingExperiment != null) searchParams.set('enable-sharding-experiment', options.enableShardingExperiment)
    if (options.hashAlg) searchParams.set('hash', options.hashAlg)
    if (options.onlyHash != null) searchParams.set('only-hash', options.onlyHash)
    if (options.pin != null) searchParams.set('pin', options.pin)
    if (options.progress) searchParams.set('progress', true)
    if (options.quiet != null) searchParams.set('quiet', options.quiet)
    if (options.quieter != null) searchParams.set('quieter', options.quieter)
    if (options.rawLeaves != null) searchParams.set('raw-leaves', options.rawLeaves)
    if (options.shardSplitThreshold) searchParams.set('shard-split-threshold', options.shardSplitThreshold)
    if (options.silent) searchParams.set('silent', options.silent)
    if (options.trickle != null) searchParams.set('trickle', options.trickle)
    if (options.wrapWithDirectory != null) searchParams.set('wrap-with-directory', options.wrapWithDirectory)
    if (options.preload != null) searchParams.set('preload', options.preload)

With this change, users who were using the hyphen based casing instead of camelCase had functionality broken. I'm not sure if this was done intentionally or deliberately, but I at least wanted to make the team aware of this.

The spec for ipfs.add and friends says the option name is wrapWithDirectory so that should be used.

I'm not sure why wrap-with-directory was supported previously - you should definitely prefer what's in the spec.

For idiomatic js we use camel case options and have always documented where possible that this style should be used for options in js-ipfs and js-ipfs-http-client. At some point in the past we also accepted kebab case but this just complicates our code and makes it more error prone. I'll PR the changelog to document the breaking change. Thanks for letting us know.

obo20 commented

No worries. It wasn't too hard of a code switch on our end. Thanks for documenting the change.