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.

Sign up for GitHub

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

Jump to bottom

chore: remove ipfs-unixfs-importer from bundle #32

Closed
wants to merge 1 commit into from
Closed

chore: remove ipfs-unixfs-importer from bundle #32

wants to merge 1 commit into from

Conversation

SgtPooki
Copy link
Member

@SgtPooki SgtPooki commented Mar 25, 2024 •
edited

Title

chore: remove ipfs-unixfs-importer from bundle

Description

fixes #31

Notes & open questions

removes ipfs-unixfs-importer from the bundle

This is potentially dangerous because we're explicitly removing it from the resulting bundle and ipfs-unixfs-importer may export some things that are needed by @helia/unixfs for non-importing cases.

However, tests seem to pass locally with this change.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

@SgtPooki SgtPooki requested a review from a team as a code owner March 25, 2024 17:51
@2color
Copy link
Member

2color commented Mar 27, 2024

Note to self:

This is affecting the external config of esbuild https://esbuild.github.io/api/#external reference : https://github.com/ipfs/aegir/blob/dc5c226c51a592d8e7a84a7143154b1a4da78097/src/types.ts#L64

@achingbrain
Copy link
Member

achingbrain commented Apr 5, 2024 •
edited

I've opened #42 as an alternative here. Instead of removing deps from the tree (which may have unpredictable results), we could just remove @helia/unixfs and use the exporter directly, since we only stat and cat a DAG we don't need all the code that deals with writing DAGs.

@SgtPooki
Copy link
Member Author

SgtPooki commented Apr 8, 2024

we should follow the approach from #42

@SgtPooki SgtPooki closed this Apr 8, 2024
This was referenced Apr 8, 2024
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@2color 2color 2color approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat: tree-shake ipfs-unixfs-importer
3 participants
@SgtPooki @2color @achingbrain