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: types path issue #99

Merged
merged 8 commits into from
Sep 27, 2021
Merged

fix: types path issue #99

merged 8 commits into from
Sep 27, 2021

Conversation

superical
Copy link
Contributor

@superical superical commented Sep 24, 2021

Summary

The contract types from typechain aren't built correctly in the path. The contract types are missing and have issues when imported into a Typescript project.

Changes

  • Corrected the build path for the types
  • Added build clean up
  • Bumped target node to v8 from v6
  • Rename and export typechain factory names automatically on build

Issues

Resolves #97 #98

@superical superical marked this pull request as ready for review September 24, 2021 08:04
@superical superical requested a review from Nebulis September 24, 2021 08:05
@superical superical linked an issue Sep 24, 2021 that may be closed by this pull request
src/index.ts Outdated
@@ -36,3 +36,5 @@ export {
NaivePaymaster__factory as NaivePaymasterFactory,
UpgradableDocumentStore__factory as UpgradableDocumentStoreFactory,
} from "./contracts";

export * from "./contracts";
Copy link
Contributor

Choose a reason for hiding this comment

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

Hum is this needed? We have renamed some export, this will lead to some inconsistency

Renaming happened after a breaking change in typechain (just for info), to. avoid a breaking in this library

Maybe there is a way to configure how factories are generated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've tried to checked on this one but it doesn't look like there is a direct way to configure how the factories are being generated.

Did you mean using the default exported names from ./contracts broke something in this library and that was why we renamed the typechain factories before exporting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wrote a little script to automate the manual process of renaming the factories from typechain on build. Hopefully this will resolve the issue with inconsistency. d2881aa

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If this is okay, then I will merge this PR and release the fix today.

Copy link
Contributor

Choose a reason for hiding this comment

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

Where is the node script called?

Copy link
Contributor

Choose a reason for hiding this comment

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

ok nvm, the post thing =)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, it's through the post thing.😁

Comment on lines -32 to -38
export {
DocumentStore__factory as DocumentStoreFactory,
DocumentStoreCreator__factory as DocumentStoreCreatorFactory,
GsnCapableDocumentStore__factory as GsnCapableDocumentStoreFactory,
NaivePaymaster__factory as NaivePaymasterFactory,
UpgradableDocumentStore__factory as UpgradableDocumentStoreFactory,
} from "./contracts";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed these as they will be renamed and exported automatically on build.

@superical superical merged commit c06abe5 into master Sep 27, 2021
@superical superical deleted the fix/build-types branch September 27, 2021 08:51
@john-dot-oa
Copy link

🎉 This PR is included in version 2.2.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

deployAndWait function confusion Cannot find module '../BaseDocumentStore' etc.....
3 participants