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

deps: use TS multiformats #2153

Closed
wants to merge 5 commits into from

Conversation

maschad
Copy link
Member

@maschad maschad commented Oct 10, 2023

do not merge

Related #2152

@maschad maschad requested a review from a team as a code owner October 10, 2023 21:47
@maschad maschad marked this pull request as draft October 10, 2023 21:48
@maschad
Copy link
Member Author

maschad commented Oct 10, 2023

@wemeetagain I am getting some module not found errors despite them appearing on the file system, I am not sure if it's related to how the types are exported

@achingbrain
Copy link
Member

I think multiformats is not being built on install? It's a GH branch dependency here so it's coming down as raw typescript, it needs to be transpiled to js before it'll work - the multiformats module needs a prepare script or smth.

Just, we need to be careful with prepare because it runs during npm i locally so if there's some ts breakage in dom.d.ts or whatever it'll break installing deps when doing local multiformats development 😩

It also pulls down all dev deps when used as a GH dependency (vs npm dependency) so can be absurdly heavy, and mean multiple versions of aegir being installed and other such 🤮 .

@maschad
Copy link
Member Author

maschad commented Oct 17, 2023

Good call @achingbrain the tests are passing now.

@wemeetagain I've updated multiformats/js-multiformats#251

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants