Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

chore: remove ipld formats re-export #872

Merged
merged 1 commit into from Oct 29, 2018
Merged

chore: remove ipld formats re-export #872

merged 1 commit into from Oct 29, 2018

Conversation

vmx
Copy link
Contributor

@vmx vmx commented Oct 9, 2018

Prior to this change the ipld-dag-cbor and ipld-dag-pb modules
are re-exported so that can be accessed within the Browser bundle.
Those modules normally don't need to be used directly, they are
kind of implementation details of IPLD. Hence remove them.

BREAKING CHANGE: remove types.dagCBOR and dag.dagPB from public API

@ghost ghost assigned vmx Oct 9, 2018
@ghost ghost added the in progress label Oct 9, 2018
@vmx vmx mentioned this pull request Oct 9, 2018
Closed
vasco-santos
vasco-santos approved these changes Oct 9, 2018
View reviewed changes
Copy link
Contributor

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM

@vmx vmx mentioned this pull request Oct 9, 2018
Merged
@vmx
Copy link
Contributor Author

vmx commented Oct 9, 2018

Those three commits should be merged together:

@vmx vmx mentioned this pull request Oct 9, 2018
Merged
lidel
lidel approved these changes Oct 9, 2018
View reviewed changes
Copy link
Contributor

@lidel lidel left a comment

Choose a reason for hiding this comment

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

LGTM

alanshaw
alanshaw suggested changes Oct 11, 2018
View reviewed changes
Copy link
Contributor

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

Could you expand on:

they are kind of implementation details of IPLD

object.put can receive instances of DagNode and there are static properties e.g. isDagNode that might be useful to users.

Is it in some way damaging to leave this as is? Does it impact bundle size - should users really not be allowed to access these two IPLD formats?

@vmx
Copy link
Contributor Author

vmx commented Oct 11, 2018

@alanshaw This change was motivated by removing ipld-dag-cbor as it isn't a direct dependency of js-ipfs, it's only there so that it can be re-exported. So I thought, before I upgrade it, I'll just remove it.

And while I was at it, I also removed ipld-dag-pb as I always try to keep a public API as small as possible. Less surface, less things to be kept stable.

@vmx
chore: remove ipld formats re-export
62b00be 
Prior to this change the `ipld-dag-cbor` and `ipld-dag-pb` modules
are re-exported so that can be accessed within the Browser bundle.
Those modules normally don't need to be used directly, they are
kind of implementation details of IPLD. Hence remove them.

BREAKING CHANGE: remove `types.dagCBOR` and `dag.dagPB` from public API

If you need the `ipld-dag-cbor` or `ipld-dag-pb` module in the Browser,
you need to bundle them yourself.
@vmx vmx force-pushed the remove-ipld-formats-re-export branch from 5e35c42 to 62b00be Compare October 12, 2018 11:38
@alanshaw
Copy link
Contributor

See ipfs-inactive/interface-js-ipfs-core#374 (review) for further rationale

@vmx
Copy link
Contributor Author

vmx commented Oct 16, 2018

@alanshaw CI passed, so I think it's ready to go :)

@alanshaw
Copy link
Contributor

Sorry I didn’t notice before but “chore:” commits don’t make it into the changelog. Should this be “refactor:” instead? Also please could you make sure the breaking change message is in the commit body.

@vmx
Copy link
Contributor Author

vmx commented Oct 17, 2018

@alanshaw The breaking change is in the commit message and it will also show up in the changelog as it is a breaking change (i've tested that locally). You can see the output if you run npx conventional-changelog -p angular -i CHANGELOG.md -s.

@alanshaw
Copy link
Contributor

Perfect, thanks. I was looking at this on mobile before and it didn't let me drill down into the commit message.

@vmx
Copy link
Contributor Author

vmx commented Oct 22, 2018

Ci is finally green! :)

@alanshaw
Copy link
Contributor

Merging as this is failing due to unrelated ping test failures.

@alanshaw alanshaw merged commit c534375 into master Oct 29, 2018
@ghost ghost removed the in progress label Oct 29, 2018
@alanshaw alanshaw deleted the remove-ipld-formats-re-export branch October 29, 2018 16:38
daviddias pushed a commit that referenced this pull request Oct 30, 2018
@hugomrdias @daviddias
fix: remove unused deps (#880)
5bd24c7 
* chore: remove ipld formats re-export (#872)

Prior to this change the `ipld-dag-cbor` and `ipld-dag-pb` modules
are re-exported so that can be accessed within the Browser bundle.
Those modules normally don't need to be used directly, they are
kind of implementation details of IPLD. Hence remove them.

BREAKING CHANGE: remove `types.dagCBOR` and `types.dagPB` from public API

If you need the `ipld-dag-cbor` or `ipld-dag-pb` module in the Browser,
you need to bundle them yourself.

* chore: update to ipld-dag-cbor 0.13

No further changes are needed as js-ipfs-api is getting the raw blocks
from IPFS and does the whole parsing client-sided in JavaScript.

BREAKING CHANGE: dag-cbor nodes now represent links as CID objects

The API for [dag-cbor](https://github.com/ipld/js-ipld-dag-cbor) changed.
Links are no longer represented as JSON objects (`{"/": "base-encoded-cid"}`,
but as [CID objects](https://github.com/ipld/js-cid). `ipfs.dag.get()` and
now always return links as CID objects. `ipfs.dag.put()` also expects links
to be represented as CID objects. The old-style JSON objects representation
is still supported, but deprecated.

Prior to this change:

```js
const cid = new CID('QmXed8RihWcWFXRRmfSRG9yFjEbXNxu1bDwgCFAN8Dxcq5')
// Link as JSON object representation
const putCid = await ipfs.dag.put({link: {'/': cid.toBaseEncodedString()}})
const result = await ipfs.dag.get(putCid)
console.log(result.value)

```

Output:

```js
{ link:
   { '/':
      <Buffer 12 20 8a…> } }
```

Now:

```js
const cid = new CID('QmXed8RihWcWFXRRmfSRG9yFjEbXNxu1bDwgCFAN8Dxcq5')
// Link as CID object
const putCid = await ipfs.dag.put({link: cid})
const result = await ipfs.dag.get(putCid)
console.log(result.value)
```

Output:

```js
{ link:
   CID {
     codec: 'dag-pb',
     version: 0,
     multihash:
      <Buffer 12 20 8a…> } }
```

See ipld/ipld#44 for more information on why this
change was made.

* chore: update deps

* fix: remove unused deps

* chore: remove socket.io, not used anymore
daviddias pushed a commit that referenced this pull request Oct 30, 2018
@hugomrdias @daviddias
fix: remove unused deps (#880)
46bbef2 
* chore: remove ipld formats re-export (#872)

Prior to this change the `ipld-dag-cbor` and `ipld-dag-pb` modules
are re-exported so that can be accessed within the Browser bundle.
Those modules normally don't need to be used directly, they are
kind of implementation details of IPLD. Hence remove them.

BREAKING CHANGE: remove `types.dagCBOR` and `types.dagPB` from public API

If you need the `ipld-dag-cbor` or `ipld-dag-pb` module in the Browser,
you need to bundle them yourself.

* chore: update to ipld-dag-cbor 0.13

No further changes are needed as js-ipfs-api is getting the raw blocks
from IPFS and does the whole parsing client-sided in JavaScript.

BREAKING CHANGE: dag-cbor nodes now represent links as CID objects

The API for [dag-cbor](https://github.com/ipld/js-ipld-dag-cbor) changed.
Links are no longer represented as JSON objects (`{"/": "base-encoded-cid"}`,
but as [CID objects](https://github.com/ipld/js-cid). `ipfs.dag.get()` and
now always return links as CID objects. `ipfs.dag.put()` also expects links
to be represented as CID objects. The old-style JSON objects representation
is still supported, but deprecated.

Prior to this change:

```js
const cid = new CID('QmXed8RihWcWFXRRmfSRG9yFjEbXNxu1bDwgCFAN8Dxcq5')
// Link as JSON object representation
const putCid = await ipfs.dag.put({link: {'/': cid.toBaseEncodedString()}})
const result = await ipfs.dag.get(putCid)
console.log(result.value)

```

Output:

```js
{ link:
   { '/':
      <Buffer 12 20 8a…> } }
```

Now:

```js
const cid = new CID('QmXed8RihWcWFXRRmfSRG9yFjEbXNxu1bDwgCFAN8Dxcq5')
// Link as CID object
const putCid = await ipfs.dag.put({link: cid})
const result = await ipfs.dag.get(putCid)
console.log(result.value)
```

Output:

```js
{ link:
   CID {
     codec: 'dag-pb',
     version: 0,
     multihash:
      <Buffer 12 20 8a…> } }
```

See ipld/ipld#44 for more information on why this
change was made.

* chore: update deps

* fix: remove unused deps

* chore: remove socket.io, not used anymore
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alanshaw alanshaw alanshaw approved these changes

@lidel lidel lidel approved these changes

@vasco-santos vasco-santos vasco-santos approved these changes

Assignees

@vmx vmx

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@vmx @alanshaw @lidel @vasco-santos