-
Notifications
You must be signed in to change notification settings - Fork 305
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: ts contracts codegen #8226
Conversation
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.
Looks good @olehmisar, thanks for the contribution! Could you just revert the verbatimModuleSyntax
addition? In a few places we target cjs, which based on the docs seem like could fail if we have that option set:
That does have some implications when it comes to module interop though. Under this flag, ECMAScript imports and exports won’t be rewritten to require calls when your settings or file extension implied a different module system. Instead, you’ll get an error.
I'm also curious as to why you were getting an error with the public override
syntax, as it's working fine within the monorepo. Maybe different ts version? Anyway, the change works well, so I'm fine with it.
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.
Looks good, thanks!
89e536a
to
9f81720
Compare
@spalladino not sure why CI is failing... |
9f81720
to
ea89fea
Compare
@olehmisar unfortunately our CI setup does not play well with contributions from external users. We have set up a job to automatically create a mirror PR under a different user so it can run CI, but it's currently failing. We'll be looking into this shortly. |
Let me close this one and manually open a new one. Sorry for the issue. |
Original PR is #8226 by @olehmisar Another attempt, this time with the branch local to this repository, instead of trying to merge across forks. --------- Co-authored-by: oleh <[email protected]>
Fixes #4152. Related #4162 and #5792.
type
keyword for type-only imports in generated contracts, so projects usingverbatimModuleSyntax: true
can typecheck successfully.public override methods!
syntax.I also added
verbatimModuleSyntax: true
to tsconfig to showcase that it fails to typecheck withouttype
prefix.