Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix(dag): check dag.put options for plain object #1480

Merged
merged 3 commits into from
Aug 8, 2018
Merged

fix(dag): check dag.put options for plain object #1480

merged 3 commits into from
Aug 8, 2018

Conversation

negamaxi
Copy link
Contributor

fixes #1479

} else if ((options.format && !options.hashAlg) || (!options.format && options.hashAlg)) {
return callback(new Error('Can\'t put dag node. Please provide `format` AND `hashAlg` options.'))
options = {}
} else if (isPlainObject(options)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about instead of introduce isPlainObject() we'd just introduce a check on whether options is defined:

if (options && options.cid && (options.format || options.hasAlg)) {
  ...
} else if (/* same thing here */) 

And then we'd remove the else block and always do sth. like options = options || {}

@0x-r4bbit
Copy link
Contributor

This LGTM but there seems to be a hiccup with CI.. maybe someone with privilege can restart the job to see if that fixes it?

@ghost ghost assigned alanshaw Aug 7, 2018
@ghost ghost added the status/in-progress In progress label Aug 7, 2018
@alanshaw
Copy link
Member

alanshaw commented Aug 7, 2018

Thanks @negamaxi ✨ . I have rebased this. Fixes to failing and temperamental tests have been merged to master so lets see what CI says now :D

License: MIT
Signed-off-by: Alan Shaw <alan@tableflip.io>
@alanshaw alanshaw merged commit d0b671b into ipfs:master Aug 8, 2018
@ghost ghost removed the status/in-progress In progress label Aug 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dag.put - Cannot read property 'cid' of undefined
3 participants