Skip to content

Conversation

mihirgupta0900
Copy link
Contributor

@mihirgupta0900 mihirgupta0900 commented Apr 8, 2023

fixes #1262

I have forked the concat-stream repository and have patched it with the changes that existed in the other forked version. This is definitely a "patch" and I think there needs to be a better solution to fix this issue.

Ideally the version for ipfs-http-client is upgraded to the latest version along with any changes that are required

@changeset-bot
Copy link

changeset-bot bot commented Apr 8, 2023

🦋 Changeset detected

Latest commit: fbe3ee4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@graphprotocol/graph-cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@mihirgupta0900 mihirgupta0900 changed the title cli: fixed dep error from ipfs-http-client and concat-stream cli: fixes dep error from ipfs-http-client and concat-stream Apr 8, 2023
Copy link
Contributor

@adamazad adamazad left a comment

Choose a reason for hiding this comment

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

This patch works in my monorepo for the time being. Thanks for pulling this together. I suggest we upgrade the package entirely — it was published 4 years ago — to avoid this technical debt.

@mihirgupta0900
Copy link
Contributor Author

mihirgupta0900 commented Apr 9, 2023

This patch works in my monorepo for the time being. Thanks for pulling this together. I suggest we upgrade the package entirely — it was published 4 years ago — to avoid this technical debt.

@adamazad
Yeah upgrading the packaging is definitely the best long term option.

FWIW I tried to upgrade the package, but because new versions of ipfs-http-client are ESM only (source: ipfs/js-ipfs#4138), which requires there to be larger changes to the code

Although pushing this patch as early as the team can would be a great help

Copy link
Contributor

@saihaj saihaj left a comment

Choose a reason for hiding this comment

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

let's patch this for now and definitely need to move away from this

@saihaj saihaj merged commit 2ba2ba9 into graphprotocol:main Apr 9, 2023
@saihaj
Copy link
Contributor

saihaj commented Apr 9, 2023

thank you @mihirgupta0900!

@saihaj
Copy link
Contributor

saihaj commented Apr 9, 2023

doesn't look like this worked in @graphprotocol/[email protected]

@fishTsai20
Copy link

not work in [@graphprotocol/[email protected]] either

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dependency Error on Installing @graphprotocol/graph-cli due to [email protected]

4 participants