-
Notifications
You must be signed in to change notification settings - Fork 184
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
fix(cli): support public library methods in modules #3308
Conversation
🦋 Changeset detectedLatest commit: dc5ac42 The changes in this PR will be included in the next version bump. This PR includes changesets to release 25 packages
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 |
4938dfb
to
b223926
Compare
7036c55
to
860af17
Compare
if (!requirePath) throw new Error("Could not find package.json to import relative to."); | ||
const require = createRequire(requirePath); | ||
|
||
const moduleOutDirs = config.modules.flatMap((mod) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe wrap this in a set so if we install multiple ERC20s, we only need to load the libs once?
const moduleOutDirs = config.modules.flatMap((mod) => { | |
const moduleOutDirs = new Set(config.modules.flatMap((mod) => { |
or ideally, we add support for multiple out dirs in findLibraries
that can do this automatically
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah makes sense, I had actually thought of deduplicating but then forgot. Will try it withing findLibraries 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/latticexyz/mud/pull/3321/files
Let me know if this looks good to you 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! thanks for the fix!
No description provided.