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

Fix patch-package error during installation #154

Merged
merged 2 commits into from
Jan 16, 2024

Conversation

compojoom
Copy link
Contributor

Fix a bug

fixes #153

Bug Report

Installing 4.0.0 was failing due to patch-package being a dev dependencies and the patches folder missing in the final pkg.

Implementation

I was thinking of adding a patch-package step to the actions workflow, but this would mean that people installing would need to extra run the step. So I decided to move patch-package to dependencies and include the patch in the dist.

This would increatese file size, but I think it is the most developer friendly solution.

Additional Context

Add any other context about the implementation here.

This PR:

  • Adds details of the module to the README (items are sorted alphabetically)
  • Adds addresses of the canonical deployments of the mastercopy for your module to deployments.json, along with addresses and ABI details to constants.ts. Ideally these should be deterministic deployments that can be easily replicated on other supported networks.
  • Includes an audit from a reputable auditor.

@compojoom compojoom changed the title Fix/patch package Fix patch-package error during installation Jan 15, 2024
Copy link
Collaborator

@jfschwarz jfschwarz left a comment

Choose a reason for hiding this comment

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

It seems like the only reason patch-package needs to be a dependency (rather than a devDependency) is that the postinstall runs on both, local dev setup install and for users installing @gnosis.pm/zodiac as a dependency.

Wondering if moving the patch-package call to the generate:types script would allow us to get rid of the extra dependency?

"generate:types": "patch-package && rm -rf src/types && typechain --target ethers-v6 --out-dir sdk/types './sdk/abi/*.json'",

@compojoom
Copy link
Contributor Author

compojoom commented Jan 15, 2024

Looks like a good idea.
I've moved it to "generate:types".

I generated a local package with:

yarn prerelease
yarn pack

then installed the generated package and it seems to work fine.

@jfschwarz jfschwarz merged commit b0265f1 into gnosisguild:master Jan 16, 2024
1 check passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2024
@compojoom compojoom deleted the fix/patch_package branch February 7, 2024 09:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Latest release is broken due to patch-package
2 participants