Skip to content

Make sure all generated code compiles#67680

Open
crisbeto wants to merge 10 commits intoangular:mainfrom
crisbeto:compliance-isolated
Open

Make sure all generated code compiles#67680
crisbeto wants to merge 10 commits intoangular:mainfrom
crisbeto:compliance-isolated

Conversation

@crisbeto
Copy link
Member

@crisbeto crisbeto commented Mar 13, 2026

These changes ensure that all generated instructions and definitions compile when passed back through TypeScript. The changes are roughly in the following categories:

  • Instructions whose parameters were too restrictive and had to be updated.
  • Generating @ts-ignore comments on some expressions that we can't easily annotate to be valid.
  • Adding explicit types (usually any) on generated variables and function parameters.

Along the way I also found a couple of issues:

  • The Ivy transform was dropping the exclamation token from property declarations.
  • In some cases, depending on the selector, we were generating a field in the component definition that isn't used for anything.

@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Mar 13, 2026
@angular-robot angular-robot bot added area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime labels Mar 13, 2026
@ngbot ngbot bot added this to the Backlog milestone Mar 13, 2026
@crisbeto crisbeto added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Mar 13, 2026
@crisbeto crisbeto force-pushed the compliance-isolated branch from 1cf3d5c to f373858 Compare March 13, 2026 13:44
@crisbeto crisbeto requested review from atscott and mattrbeck March 13, 2026 13:44
@crisbeto crisbeto marked this pull request as ready for review March 13, 2026 13:44
const testFiles = loadStandardTestFiles();

function cleanNewLines(contents: string) {
return contents.replace(/\n/g, ' ').replace(/\s+/g, ' ');
Copy link
Member

Choose a reason for hiding this comment

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

The .replace(/\s+/g, ' ') probably works for our case, but if we're just trying to replace newlines then maybe .replace(/\s*\n\s*/g, ' ') is a little safer?

crisbeto added 10 commits March 13, 2026 18:27
Updates the signatures of generated code to match what the compiler actually generates.
Fixes that the compiler was generating an `attrs` field on the definition which isn't used anywhere.
Fixes that the Ivy transform was dropping the `exclamationToken` from properties.
Updates the output AST to allow leading comments to be attached to expression nodes.
…e tests

Updates the compliance tests to check if the generated code compiles.
Adds explicit type annotations to the generated code so it's able to compile.
Adds `@ts-ignore` comments to same places in the pipeline where we can't produce code that passes all type checks.
In some cases the `debugName` transform generates a spread into the signal function parameters. This can cause compiler errors, because the functions don't have rest parameters.

These changes work around it by adding a `@ts-ignore` above it.
Renames the test target so it's a bit clearer what it's targeting.
Updates the compliance tests to account for the recent changes.
@crisbeto crisbeto force-pushed the compliance-isolated branch from f373858 to a24918b Compare March 13, 2026 17:28
Copy link
Contributor

@atscott atscott left a comment

Choose a reason for hiding this comment

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

personal preference/consider nit: I feel like it would have been nice to regenerate the compliance goldens in the commits that affected the outputs. That way it would have been really easy to see what the outcome of each individual change was (and would make each individual commit continue to pass the tests)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: review The PR is still awaiting reviews from at least one requested reviewer area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants