feat!: replace `ipld-dag-pb` with `@ipld/dag-pb` #330

SgtPooki · 2022-07-09T01:43:02Z

~~FYI: Much of this was me scrambling in the dark without knowing 1/4th of what I'm doing. IDK what:~~

~~1. the correct interface is supposed to be (the docs and source-code of dependencies are out of date or otherwise incorrect)~~
~~2. what exactly ipld/pb/cbor is supposed to do, especially in this repo~~
~~3. Any and everything else you may assume I know~~

~~With that said,~~ comments & feedback that are typically given noobs and to the otherwise clueless are more than welcome and honestly expected.

fixes #329

~~I could use some eyes from the experts in this space. @achingbrain @lidel @mikeal~~

SgtPooki · 2022-07-09T01:56:30Z

src/lib/cid.test.js

+
+const { CID, hasher, bytes } = multiformats
+
+export const sha256 = hasher.from({


sha256 is not exported from multiformats correctly even though it says I can import it from multiformats/hashes/sha2

import { sha256 } from 'multiformats/hashes/sha2'

if the docs are wrong, a PR would be appreciated, but the export is there and we actively use it in many places

That import doesn't even work :(

what runtime or bundler is this using? perhaps something needs to be updated here to support proper export maps?

react@16.14 is probably the culprit here, I don't think they'd nailed down their export maps functionality quite right back then but I don't know why this import specifically would fail tbh

but I bet upgrading react here would be a major pain too! so go with whatever works I suppose.

I'm assuming this has something to do with migrating js-ipfs stuff to esm, but the GUI packages all still being cjs

src/lib/normalise-dag-node.js

SgtPooki

self-review completed.. trivial errors and unnecessary comments should be gone

src/lib/cid.js

rvagg · 2022-07-12T04:24:58Z

I don't understand the TextEncoder & TextDecoder problem here, why is it borking on those such that they need to be imported from 'util'? They've been standard globals for quite a while now: https://developer.mozilla.org/en-US/docs/Web/API/TextDecoder/TextDecoder#browser_compatibility

Do we need to update runtimes used in CI here or something?

hacdias · 2022-07-15T09:20:24Z

@SgtPooki maybe I missed it, but is there any reason to not use ipld/js-blockcodec-to-ipld-format to convert the new format to the old format (and therefore be supported here) like it was done in #319?

rvagg · 2022-07-19T22:26:41Z

@RangerMauve ping on the export maps / react / tooling discussion - here's a pain-point I think, see discussion inline about sha256.

RangerMauve · 2022-07-19T22:28:08Z

Gonna look into this early next week as part of the ESM migration stuff.

SgtPooki · 2022-08-29T21:59:47Z

@SgtPooki maybe I missed it, but is there any reason to not use ipld/js-blockcodec-to-ipld-format to convert the new format to the old format (and therefore be supported here) like it was done in #319?

I would say the main reason is that it leaves our code in a state of perpetual deprecation. I'll try it just to keep the code-changes down, but we still need a better story for moving forward.

Also, Instead of us maintaining something like https://github.com/ipld/js-blockcodec-to-ipld-format (last update 16 months ago), creating a tool that migrates old code to the new code would be much better.

SgtPooki · 2022-08-29T22:09:41Z

I don't understand the TextEncoder & TextDecoder problem here, why is it borking on those such that they need to be imported from 'util'? They've been standard globals for quite a while now: https://developer.mozilla.org/en-US/docs/Web/API/TextDecoder/TextDecoder#browser_compatibility

Do we need to update runtimes used in CI here or something?

it's a jsdom thing. I believe I have a workaround. See jsdom/jsdom#2524

BigLep · 2022-09-12T21:08:12Z

@rvagg : can you give feedback on whether @SgtPooki is doing things right/correct now?

src/lib/cid.js

src/lib/normalise-dag-node.js

Co-authored-by: Rod Vagg <rod@vagg.org>

SgtPooki · 2022-09-26T22:28:50Z

ping for review from @lidel and @rvagg

hacdias

Codewise, it looks good to me. However, I would like to actually try running this, perhaps using https://github.com/ipld/explore.ipld.io to make sure it works properly. Sadly, I did not manage to make it work with either npm link, or npx link. Besides the storybooks, how did you try this out?

ipfs/ipld-explorer-components#330

SgtPooki · 2022-10-04T19:50:47Z

Codewise, it looks good to me. However, I would like to actually try running this, perhaps using https://github.com/ipld/explore.ipld.io to make sure it works properly. Sadly, I did not manage to make it work with either npm link, or npx link. Besides the storybooks, how did you try this out?

I just used storybook. I just attempted https://github.com/ipld/explore.ipld.io too, but that will require updating ipld-dag-pb in that repo as well. Getting it to work in that repo just to prove that it works here (which it does, fine, in storybook) seems excessive. If we have to do that for every change needed to resolve ipfs/ipfs-webui#1965 then that issue will be the death of webui.

hacdias

You're right, let's do it!

SgtPooki · 2022-10-04T22:35:16Z

@hacdias marked PR title with feat! instead of chore: to indicate that this is a breaking change. thanks for the callout

hacdias · 2022-10-05T08:57:26Z

Great. I'm fine to merge this.

## [3.0.0](v2.6.1...v3.0.0) (2022-10-05) ### ⚠ BREAKING CHANGES * replace `ipld-dag-pb` with `@ipld/dag-pb` (#330) ### Features * replace `ipld-dag-pb` with `@ipld/dag-pb` ([#330](#330)) ([8cd0b23](8cd0b23))

github-actions · 2022-10-05T17:45:45Z

🎉 This PR is included in version 3.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

SgtPooki added 8 commits July 8, 2022 14:45

feat: remove ipld-dag-* deps

5e251d1

feat: add cid.js tests

63cd3fa

feat: finish getCodeOrNull tests

565f405

feat: full coverage for src/lib/cid.js

28062ed

chore: fixing older tests (in progress)

56e1a9d

fix: resolve nodes traversed along path test

a2f9486

feat: all tests pass

b141678

chore: add .npmrc and patches

4e47547

SgtPooki linked an issue Jul 9, 2022 that may be closed by this pull request

Replace ipld-dag-pb with @ipld/dag-pb #329

Closed

SgtPooki added 2 commits July 8, 2022 18:50

fix: remove package.json regressions

6aeee79

fix: @ipld/dag-pb patch update

b660f56

SgtPooki commented Jul 9, 2022

View reviewed changes

src/lib/normalise-dag-node.js Show resolved Hide resolved

chore: self review / code cleanup

ef68de3

SgtPooki commented Jul 9, 2022

View reviewed changes

SgtPooki requested review from lidel and hacdias July 9, 2022 02:07

SgtPooki mentioned this pull request Jul 9, 2022

feat: update ipfs-http-client ipfs/ipfs-webui#1840

Closed

5 tasks

SgtPooki self-assigned this Jul 11, 2022

rvagg reviewed Jul 12, 2022

View reviewed changes

src/lib/cid.js Show resolved Hide resolved

SgtPooki added 2 commits August 29, 2022 15:49

fix: define textEncoder+Decoder in jsdom

a8a51ac

fix: remove patches

984e8ff

BigLep requested a review from rvagg September 12, 2022 21:04

SgtPooki mentioned this pull request Sep 12, 2022

chore: Dependency updates ipfs/ipfs-webui#1965

Open

23 tasks

rvagg reviewed Sep 13, 2022

View reviewed changes

src/lib/cid.js Outdated Show resolved Hide resolved

rvagg reviewed Sep 13, 2022

View reviewed changes

src/lib/normalise-dag-node.js Outdated Show resolved Hide resolved

rvagg reviewed Sep 13, 2022

View reviewed changes

src/lib/normalise-dag-node.js Outdated Show resolved Hide resolved

rvagg reviewed Sep 13, 2022

View reviewed changes

src/lib/normalise-dag-node.js Show resolved Hide resolved

SgtPooki requested a review from a team as a code owner September 15, 2022 20:40

SgtPooki and others added 4 commits September 15, 2022 13:40

Update src/lib/cid.js

c0f64aa

Co-authored-by: Rod Vagg <rod@vagg.org>

Update src/lib/normalise-dag-node.js

f5fd93c

Co-authored-by: Rod Vagg <rod@vagg.org>

Merge branch 'master' into 329-replace-ipld-dag-pb-with-iplddag-pb

73bd252

fix: errors in src/lib/cid.js

ffbef07

SgtPooki changed the title ~~chore: replace ipld-dag-pb with @ipld/dag-pb~~ [WIP] chore: replace ipld-dag-pb with @ipld/dag-pb Sep 23, 2022

SgtPooki added 4 commits September 24, 2022 17:38

chore: update ipfs webpack bundle

b68003f

chore: create types.d.ts

58841d7

chore: cleanup old comments and CIDs

2b842a4

chore: remove unnecessary data manipulation

3448e86

SgtPooki changed the title ~~[WIP] chore: replace ipld-dag-pb with @ipld/dag-pb~~ chore: replace ipld-dag-pb with @ipld/dag-pb Sep 29, 2022

hacdias reviewed Oct 4, 2022

View reviewed changes

SgtPooki added a commit to SgtPooki/explore.ipld.io that referenced this pull request Oct 4, 2022

tmp: demo for PR

b503fc7

ipfs/ipld-explorer-components#330

hacdias approved these changes Oct 4, 2022

View reviewed changes

SgtPooki changed the title ~~chore: replace ipld-dag-pb with @ipld/dag-pb~~ !feat: replace ipld-dag-pb with @ipld/dag-pb Oct 4, 2022

SgtPooki changed the title ~~!feat: replace ipld-dag-pb with @ipld/dag-pb~~ feat!: replace ipld-dag-pb with @ipld/dag-pb Oct 4, 2022

SgtPooki merged commit 8cd0b23 into master Oct 5, 2022

SgtPooki deleted the 329-replace-ipld-dag-pb-with-iplddag-pb branch October 5, 2022 17:42

github-actions bot added the released label Oct 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: replace `ipld-dag-pb` with `@ipld/dag-pb` #330

feat!: replace `ipld-dag-pb` with `@ipld/dag-pb` #330

SgtPooki commented Jul 9, 2022 •

edited

SgtPooki Jul 9, 2022

rvagg Jul 12, 2022 •

edited

SgtPooki Jul 18, 2022

rvagg Jul 18, 2022

rvagg Jul 18, 2022

SgtPooki Aug 29, 2022

SgtPooki left a comment

rvagg commented Jul 12, 2022

hacdias commented Jul 15, 2022

rvagg commented Jul 19, 2022

RangerMauve commented Jul 19, 2022

SgtPooki commented Aug 29, 2022

SgtPooki commented Aug 29, 2022

BigLep commented Sep 12, 2022

SgtPooki commented Sep 26, 2022

hacdias left a comment

SgtPooki commented Oct 4, 2022

hacdias left a comment

SgtPooki commented Oct 4, 2022

hacdias commented Oct 5, 2022

github-actions bot commented Oct 5, 2022


		const { CID, hasher, bytes } = multiformats

		export const sha256 = hasher.from({

feat!: replace ipld-dag-pb with @ipld/dag-pb #330

feat!: replace ipld-dag-pb with @ipld/dag-pb #330

Conversation

SgtPooki commented Jul 9, 2022 • edited

SgtPooki Jul 9, 2022

Choose a reason for hiding this comment

rvagg Jul 12, 2022 • edited

Choose a reason for hiding this comment

SgtPooki Jul 18, 2022

Choose a reason for hiding this comment

rvagg Jul 18, 2022

Choose a reason for hiding this comment

rvagg Jul 18, 2022

Choose a reason for hiding this comment

SgtPooki Aug 29, 2022

Choose a reason for hiding this comment

SgtPooki left a comment

Choose a reason for hiding this comment

rvagg commented Jul 12, 2022

hacdias commented Jul 15, 2022

rvagg commented Jul 19, 2022

RangerMauve commented Jul 19, 2022

SgtPooki commented Aug 29, 2022

SgtPooki commented Aug 29, 2022

BigLep commented Sep 12, 2022

SgtPooki commented Sep 26, 2022

hacdias left a comment

Choose a reason for hiding this comment

SgtPooki commented Oct 4, 2022

hacdias left a comment

Choose a reason for hiding this comment

SgtPooki commented Oct 4, 2022

hacdias commented Oct 5, 2022

github-actions bot commented Oct 5, 2022

feat!: replace `ipld-dag-pb` with `@ipld/dag-pb` #330

feat!: replace `ipld-dag-pb` with `@ipld/dag-pb` #330

SgtPooki commented Jul 9, 2022 •

edited

rvagg Jul 12, 2022 •

edited