Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve findAndReplaceDagCborLinks function #338

Open
SgtPooki opened this issue Sep 26, 2022 · 0 comments
Open

Improve findAndReplaceDagCborLinks function #338

SgtPooki opened this issue Sep 26, 2022 · 0 comments
Labels
kind/enhancement A net-new feature or improvement to an existing feature kind/maintenance Work required to avoid breaking changes or harm to project's status quo need/analysis Needs further analysis before proceeding

Comments

@SgtPooki
Copy link
Member

SgtPooki commented Sep 26, 2022

This function is a bit of a mess (not your mess, this is ancient stuff), there's also the fact that your fix for the 'object' check is going to avoid arrays now, so the bug was hiding issues that already exists.

We have this logic tested and used in js-multiformats, so maybe you could use that instead? if you import multiformats/block' then you could use links()on a block form of a decoded object, and that would cover any object type regardless of codec (the fact that this is dedicated to dag-cbor is a bit weird sinceobj` could be anything—probably a historical thing but the logic in here seems to differentiate dag-pb from everything else, which is the appropriate way to deal with dag-pb since it's legacy and everything else is newer).

See multiformats/js-multiformats@master/test/test-block.js#L45-L51 for this in action, it'll properly walk through the object and yield the links in the object along with the paths. You could wrap that in the additional stuff you need to get your size etc.

Alternatively, maybe steal some of the logic in there because we've worked on that one (and tree() to better handle all possible node types and iterate through them - bytes, null, arrays, CIDs, they all come with special cases which is annoying to have to reimplement and test for yourself. multiformats/js-multiformats@f2664fb/src/block.js#L49-L68

See https://github.com/ipfs/ipld-explorer-components/pull/330/files#r969207674

@SgtPooki SgtPooki added the need/triage Needs initial labeling and prioritization label Sep 26, 2022
@SgtPooki SgtPooki added kind/enhancement A net-new feature or improvement to an existing feature kind/maintenance Work required to avoid breaking changes or harm to project's status quo need/analysis Needs further analysis before proceeding and removed need/triage Needs initial labeling and prioritization labels Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A net-new feature or improvement to an existing feature kind/maintenance Work required to avoid breaking changes or harm to project's status quo need/analysis Needs further analysis before proceeding
Projects
No open projects
Status: Needs Prep Work / Blocked
Development

No branches or pull requests

1 participant