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

[WIP] chore: update IPLD to latest version #1652

Closed
wants to merge 1 commit into from
Closed

Conversation

vmx
Copy link
Member

@vmx vmx commented Oct 22, 2018

js-ipld 0.19 has breaking changes that affect this project. Therefore
also update the corresponding code.

This is a WIP and will be force-updated once js-ipld 0.19 was released.

@alanshaw alanshaw changed the title WIP: chore: update IPLD to latest version [WIP] chore: update IPLD to latest version Oct 22, 2018
@ghost ghost assigned vmx Oct 22, 2018
@ghost ghost added the status/in-progress In progress label Oct 22, 2018
Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

Did you check if the examples still work?

It is now possible to pass in options for IPLD. This makes it possible
to pass in the formats that IPFS should support.

With version 0.19.0 of js-ipld the default formats are *only* [dag-cbor]
and [dag-pb].

BREAKING CHANGE: js-ipfs supports dag-cbor and dag-pb by default only

If you use js-ipfs as a library, only [dag-cbor] and [dag-pb] are bundled
by default. If you want to use additional formats, you need to pass them
into the constructor.

This example code shows how to get the same behaviour as before:

```js
const ipldBitcoin = require('ipld-bitcoin')
const ipldDagCbor = require('ipld-dag-cbor')
const ipldDagPb = require('ipld-dag-pb')
const ipldEthAccountSnapshot = require('ipld-ethereum').ethAccountSnapshot
const ipldEthBlock = require('ipld-ethereum').ethBlock
const ipldEthBlockList = require('ipld-ethereum').ethBlockList
const ipldEthStateTrie = require('ipld-ethereum').ethStateTrie
const ipldEthStorageTrie = require('ipld-ethereum').ethStorageTrie
const ipldEthTrie = require('ipld-ethereum').ethTxTrie
const ipldEthTx = require('ipld-ethereum').ethTx
const ipldGit = require('ipld-git')
const ipldRaw = require('ipld-raw')
const ipldZcash = require('ipld-zcash')

const node = new IPFS({
  ipld: {
    formats: [
      ipldBitcoin, ipldDagCbor, ipldDagPb, ipldEthAccountSnapshot,
      ipldEthBlock, ipldEthBlockList, ipldEthStateTrie, ipldEthStorageTrie,
      ipldEthTrie, ipldEthTx, ipldGit, ipldRaw, ipldZcash
    ]
  }
})
```

There is no change on the js-ipfs command line tool, it still supports
all those formats.

[dag-cbor]: https://github.com/ipld/js-ipld-dag-cbor
[dag-pb]: https://github.com/ipld/js-ipld-dag-pp
@vmx
Copy link
Member Author

vmx commented Oct 24, 2018

I've checked the examples (the only one I couldn't get working was the circuit one, but after looking at the code, it should be affected by my changes), they work, resp. I made them work.

@vmx
Copy link
Member Author

vmx commented Oct 24, 2018

WARNING: This is not ready to be merged (even if CI is green). This needs a new js-ipld release first.

} else if (this._options.ipld.blockService === undefined) {
this._options.ipld.blockService = ipldDefaults.blockService
}
this._ipld = new Ipld(this._options.ipld)
Copy link
Member

Choose a reason for hiding this comment

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

Does this look better ?

    this._ipld = new Ipld(Object.assign({}, { formats: [ipldDagPb] }, this._options.ipld, { blockService: this._blockService }  ))

Copy link
Member Author

Choose a reason for hiding this comment

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

I've put your code in a file:

'use strict'

const ipldDagPb = 'ipldDagPb'
const _options = {ipld: {blockService: 'passed in block service'}}
const _blockService = 'blockService'

const result = Object.assign(
  {},
  { formats: [ipldDagPb] },
  _options.ipld,
  { blockService: _blockService }
)

console.log(result)

The out put is { formats: [ 'ipldDagPb' ], blockService: 'blockService' }, but I would expect it to be
{ formats: [ 'ipldDagPb' ], blockService: 'passed in block service' }

Copy link
Member

Choose a reason for hiding this comment

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

this is correct for js-ipfs! we need to force the already instantiated blockservice or we will need to deal with the repo instantiation.

ipld blockservice option is an instance and not a reference, and blockservice needs a repo, but ipfs already creates a repo in the instantiation/init/start lifecycle. This will get complex very fast lol :)

i think we should force blockservice and maybe in a future PR handle all the blockservice/repo instances sync with ipld config

@alanshaw alanshaw mentioned this pull request Oct 25, 2018
38 tasks
@vmx
Copy link
Member Author

vmx commented Oct 25, 2018

This PR is superseded by: #1668.

@vmx vmx closed this Oct 25, 2018
@ghost ghost removed the status/in-progress In progress label Oct 25, 2018
@vmx vmx deleted the update-ipld-0.19 branch October 25, 2018 13:06
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.

3 participants