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: typegen generic enums and ./common import (predicates, scripts) #2368

Merged

Conversation

nedsalk
Copy link
Contributor

@nedsalk nedsalk commented May 22, 2024

I found this issue after running pnpm tsc --noEmit on typegen outputs of our fuel-gauge test suite.

@nedsalk nedsalk added the bug Issue is a bug label May 22, 2024
@nedsalk nedsalk self-assigned this May 22, 2024
@nedsalk nedsalk enabled auto-merge (squash) May 22, 2024 10:28
@nedsalk nedsalk disabled auto-merge May 22, 2024 10:31
@nedsalk nedsalk marked this pull request as draft May 22, 2024 10:36
@nedsalk nedsalk changed the title fix: typegen predicates and scripts not importing from ./common fix: typegen generic enums and ./common import for predicates and scripts May 22, 2024
@nedsalk nedsalk marked this pull request as ready for review May 22, 2024 10:47
@nedsalk nedsalk changed the title fix: typegen generic enums and ./common import for predicates and scripts fix: typegen generic enums and ./common import (predicates, scripts) May 22, 2024
@nedsalk nedsalk marked this pull request as draft May 22, 2024 10:55
@nedsalk

This comment was marked as outdated.

@nedsalk nedsalk marked this pull request as ready for review May 22, 2024 11:28
Copy link
Contributor

Coverage Report:

Lines Branches Functions Statements
79.76%(+0%) 70.69%(+0%) 76.86%(+0%) 79.85%(+0%)
Changed Files:

Coverage values did not change👌.

Copy link
Contributor

@petertonysmith94 petertonysmith94 left a comment

Choose a reason for hiding this comment

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

I'm finding it difficult to validate these changes - do you have a branch for the typegen work on fuel-gauge?

@nedsalk
Copy link
Contributor Author

nedsalk commented May 22, 2024

@petertonysmith94

  1. Go to master,
  2. Run pnpm pretest --force,
  3. Run pnpm tsc --noEmit packages/fuel-gauge/test/typegen/**/*.ts

It will list out the problematic files. You'll find that the predicates and scripts that have types from common.d.ts are having errors because the import statement is missing.

Checkout this branch and do the same and it'll be solved.

Copy link
Contributor

@petertonysmith94 petertonysmith94 left a comment

Choose a reason for hiding this comment

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

@petertonysmith94

  1. Go to master,
  2. Run pnpm pretest --force,
  3. Run pnpm tsc --noEmit packages/fuel-gauge/test/typegen/**/*.ts

It will list out the problematic files. You'll find that the predicates and scripts that have types from common.d.ts are having errors because the import statement is missing.

Checkout this branch and do the same and it'll be solved.

Thanks for that, not sure what was going on there but my typegen folder wasn't being generated, I believe the --force fixed the issue.

Nice one 🥇

@nedsalk nedsalk enabled auto-merge (squash) May 22, 2024 15:55
@nedsalk nedsalk merged commit ceef79a into master May 23, 2024
19 checks passed
@nedsalk nedsalk deleted the ns/fix/typegen-common-imports-for-predicates-and-scripts branch May 23, 2024 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants