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

🚀 Feature Request: Add a command to check that the generated types are in sync #6575

Closed
stof opened this issue Aug 23, 2024 · 4 comments · Fixed by #6329
Closed

🚀 Feature Request: Add a command to check that the generated types are in sync #6575

stof opened this issue Aug 23, 2024 · 4 comments · Fixed by #6329
Labels
enhancement New feature or request

Comments

@stof
Copy link
Contributor

stof commented Aug 23, 2024

Describe the solution

In my CI job, I'd like to have a check that the worker-configuration.d.ts file is actually in sync with the worker configuration (i.e. that devs changing the configuration don't forget to run npm run cf-typegen) so that typescript type checking can be relied on.

I attempted to create a GHA job running npm run cf-typegen to regenerate the file and then using https://github.com/marketplace/actions/verify-changed-files to make the job fail if the file is modified. However, this fails because the output of wrangler types is not reproducible. The output contains a comment with the timestamp of the generation run so the file would be modified all the time in my CI job.

I see 2 ways to solve that:

  • provide a dedicated check mode in wrangler types (for instance wrangler types --check) which would compare the existing file content with the generated content with special logic to ignore changes in the timestamp, and report differences by a failure exit code
  • provide a way to make the wrangler types output reproducible (either always or with a dedicated flag) by removing the timestamp, which would allow to use existing tooling checking for modified files after running it again.
@stof stof added the enhancement New feature or request label Aug 23, 2024
@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk Aug 23, 2024
@andyjessop
Copy link
Contributor

This sounds like it would be caught by just running tsc. If your Worker code does not match your generated types, then tsc will error. Perhaps you can try running that in your CI job?

We did talk about a wrangler types --check command recently, but we reasoned that all it would be doing is:

  1. Run wrangler types.
  2. Run tsc.

@andyjessop andyjessop added the awaiting reporter response Needs clarification or followup from OP label Aug 26, 2024
@stof
Copy link
Contributor Author

stof commented Aug 26, 2024

Regenerating the wrangler types before doing typechecking in CI could work to ensure that tsc uses the right type declarations in CI, but this would still create confusion for other devs in the team which would see typescript errors locally.

I already have a CI job running tsc in CI.
However, my goal is to enforce that the version-controlled worker-configuration.d.ts matches the version-controlled wrangler.toml

Note that my request for wrangler types --check is not for it to run type checking on the whole project. It is to check whether the generated worker-configuration.d.ts matches what wrangler types would generate based on the current wrangler.toml.
And if the type generation were reproducible, I would already be able to check that by running wrangler types followed by git status (or a similar git-based detection). My blockage is that wrangler types is not reproducible due to the generation timestamp included in the comment.

@penalosa
Copy link
Contributor

@stof this should be fixed by #6329, which will make wrangler types reproducible

@stof
Copy link
Contributor Author

stof commented Aug 27, 2024

great news. This would indeed solve my need.

@lrapoport-cf lrapoport-cf added awaiting Cloudflare response Awaiting response from workers-sdk maintainer team and removed awaiting reporter response Needs clarification or followup from OP labels Sep 16, 2024
@andyjessop andyjessop removed the awaiting Cloudflare response Awaiting response from workers-sdk maintainer team label Sep 16, 2024
@github-project-automation github-project-automation bot moved this from Untriaged to Done in workers-sdk Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants