fix: updates ipld-dag-pb dep to version without .cid or .multihash properties #388
Conversation
ghost
assigned 
achingbrain
changed the title
achingbrain
force-pushed
the
update-dag-pb-to-not-have-cid-property
branch
from
9b868e6
to
5c718a6
Compare
| @@ -6,6 +6,9 @@ | |||
| "main": "js/src/index.js", | |||
| "scripts": { | |||
| "test": "exit 0", | |||
| "test:node": "exit 0", | |||
| "test:browser": "exit 0", | |||
| "test:webworker": "exit 0", | |||
There was a problem hiding this comment.
These are so Jenkins doesn't fail the build for not having tests.
See https://github.com/ipfs/jenkins-libs/blob/master/vars/javascript.groovy#L328-L330 and https://github.com/ipfs/jenkins-libs/blob/master/vars/javascript.groovy#L167-L170
There was a problem hiding this comment.
Code changes LGTM. Can we talk about the ramifications of this on the object API?
@achingbrain observed that by not having a cid/multihash property on a DAGNode the object API is significantly less useful for writes - you get basically get back the data you put in (which you aready have), but have no way to address it without calculating the CID yourself.
This means people using the object API will now need to depend on the ipld-dag-pb module and use the util.cid to calculate it.
@diasdavid I don't know how legacy we consider the object API to be - do you think this is this an acceptable breaking change since it's effectively a deprecated API anyway? I really don't want to stand in the way of these changes because of the perf gains they'll bring but this is a big breaking change with a difficult workaround.
As a suggestion, (still a breaking change) but why don't we change the write methods of the object API to return a CID? Then at least users can retrieve a DAGNode using object.get without having to require 'ipld-dag-pb' and calculate their own CID...
Presumably this would still allow reads to be fast (no recalc the CID for get) but would retain some usefulness in the object API.
|
I believe that (unfortunately) there are a considerable number of users using this API, perhaps due to a lack of "deprecation warning"
It seems to me that this approach + a good deprecation plan is the way to go. |
Also because the DAG API isn't implemented over http so you can only use it in JS if you are calling directly into core. |
There is a way to overcome this - ipfs-inactive/js-ipfs-http-client#755 (comment) |
Sounds good? |
|
Can we merge this one & I'll put a PR together for returning CIDs from the Object APIs? |
|
..The reason is I'd like to use a pre-release IPFS on npm-on-ipfs in order to verify that this alleviates the CPU pressure. |
Since `ipld-dag-pb` removed `multihash` and `cid` properties, the Object API became less useful, returning the data you provided it when you call methods that write DAG nodes. This meant the user had to calculate the CID manually. This PR updates the Object API so that write methods return CID instances instead of DAG nodes. See discussion here for more context: #388 (review) License: MIT Signed-off-by: Alan Shaw <[email protected]>
Since `ipld-dag-pb` removed `multihash` and `cid` properties, the Object API became less useful, returning the data you provided it when you call methods that write DAG nodes. This meant the user had to calculate the CID manually. This PR updates the Object API so that write methods return CID instances instead of DAG nodes. See discussion here for more context: #388 (review) License: MIT Signed-off-by: Alan Shaw <[email protected]>
Since `ipld-dag-pb` removed `multihash` and `cid` properties, the Object API became less useful, returning the data you provided it when you call methods that write DAG nodes. This meant the user had to calculate the CID manually. This PR updates the Object API so that write methods return CID instances instead of DAG nodes. See discussion here for more context: #388 (review) License: MIT Signed-off-by: Alan Shaw <[email protected]>
For the back story on this change, please see: ipfs-inactive/interface-js-ipfs-core#388 (review) BREAKING CHANGE: Object API refactor. Object API methods that write DAG nodes now return a [CID](https://www.npmjs.com/package/cids) instead of a DAG node. Affected methods: * `ipfs.object.new` * `ipfs.object.patch.addLink` * `ipfs.object.patch.appendData` * `ipfs.object.patch.rmLink` * `ipfs.object.patch.setData` * `ipfs.object.put` Example: ```js // Before const dagNode = await ipfs.object.new() ``` ```js // After const cid = await ipfs.object.new() // now returns a CID const dagNode = await ipfs.object.get(cid) // fetch the DAG node that was created ``` IMPORTANT: `DAGNode` instances, which are part of the IPLD dag-pb format have been refactored. These instances no longer have `multihash`, `cid` or `serialized` properties. This effects the following API methods that return these types of objects: * `ipfs.object.get` * `ipfs.dag.get` See ipld/js-ipld-dag-pb#99 for more information. License: MIT Signed-off-by: Alan Shaw <[email protected]>
For the back story on this change, please see: ipfs-inactive/interface-js-ipfs-core#388 (review) BREAKING CHANGE: Object API refactor. Object API methods that write DAG nodes now return a [CID](https://www.npmjs.com/package/cids) instead of a DAG node. Affected methods: * `ipfs.object.new` * `ipfs.object.patch.addLink` * `ipfs.object.patch.appendData` * `ipfs.object.patch.rmLink` * `ipfs.object.patch.setData` * `ipfs.object.put` Example: ```js // Before const dagNode = await ipfs.object.new() ``` ```js // After const cid = await ipfs.object.new() // now returns a CID const dagNode = await ipfs.object.get(cid) // fetch the DAG node that was created ``` IMPORTANT: `DAGNode` instances, which are part of the IPLD dag-pb format have been refactored. These instances no longer have `multihash`, `cid` or `serialized` properties. This effects the following API methods that return these types of objects: * `ipfs.object.get` * `ipfs.dag.get` See ipld/js-ipld-dag-pb#99 for more information. License: MIT Signed-off-by: Alan Shaw <[email protected]>
Follows on from ipld/js-ipld-dag-pb#99 and updates this module to not rely on DAGNodes having knowledge of their CIDs.