Skip to content

fix(compiler-cli): report errors when initializer APIs (input etc.) are used on private fields#54981

Closed
devversion wants to merge 6 commits intoangular:mainfrom
devversion:access-enforce-initializer-api
Closed

fix(compiler-cli): report errors when initializer APIs (input etc.) are used on private fields#54981
devversion wants to merge 6 commits intoangular:mainfrom
devversion:access-enforce-initializer-api

Conversation

@devversion
Copy link
Member

See individual commits

@devversion devversion requested a review from crisbeto March 21, 2024 17:11
@devversion devversion 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 area: compiler Issues related to `ngc`, Angular's template compiler labels Mar 21, 2024
@devversion devversion marked this pull request as ready for review March 21, 2024 17:11
@ngbot ngbot bot modified the milestone: Backlog Mar 21, 2024
@devversion devversion force-pushed the access-enforce-initializer-api branch from 93aedbd to 98b1693 Compare March 22, 2024 13:21
@devversion devversion requested a review from crisbeto March 22, 2024 13:21
@devversion
Copy link
Member Author

Note: This is a new error code for the compiler which shouldn't need public API approval

@devversion devversion added action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release and removed 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 22, 2024
crisbeto and others added 6 commits March 27, 2024 12:02
Updates the function that parses initializer APIs to check any `Expression`, instead of expecting a class member. This will be useful for the upcoming changes.
…nd visibility

This commit changes the TypeScript reflection host to:

* inspect / process ES private fields. e.g. `#someField` — those are
  ignored right now and we would want to check them to issue
  diagnostics.

* determine an access level of a class member. E.g. a member may be
  public, may be private, may be ES private, or public readonly. This
  can then be used in various checks later.
…r APIs

An initializer API like `input`, `output`, or signal queries may not be
compatible with certain access levels. E.g. queries cannot work with ES
private class fields.

This commit introduces a check for access levels into the initializer
API recognition— enforcing that every initializer API *clearly*
specifies what type of access is allowed.
…ivate fields

This commit ensures that the new APIs like `input`, `model`, `output`,
or signal-based queries are not accidentally used on fields that have a
problematic visibility/access level that won't work.

For example, queries defined using a private identifier (e.g. `#bla`)
will not be accessible by the Angular runtime and therefore _dont_ work.

This commit ensures:

- `input` is only declared via public and protected fields.
- `output` is only declared via public and protected fields.
- `model` is only declared via public and protected fields.
- signal queries are only declared via public, protected and TS private
  fields (`private` works, while `#bla` does not).

Fixes angular#54863.
…nosticError`

For `FatalDiagnosticError` we are currently hiding the `message` string
field in favor of the actual TS `diagnosticMessage`.

This works as expected, but makes these errors hard to debug in certain
environments (e.g. Jasmine). That is because `null` is the value of
`message` at runtime. We fix this by just overriding the type, like we
originally intended to do.

In addition, we properly render message chains in the `Error#message`
field— so that these errors, when uncaught, are somewhat reasonable and
can be useful.
@devversion devversion force-pushed the access-enforce-initializer-api branch from d39c6b1 to 9c4a030 Compare March 27, 2024 12:03
@dylhunn
Copy link
Contributor

dylhunn commented Mar 27, 2024

This PR was merged into the repository by commit 03b1ac3.

@dylhunn dylhunn closed this in 6b725c9 Mar 27, 2024
dylhunn pushed a commit that referenced this pull request Mar 27, 2024
…nd visibility (#54981)

This commit changes the TypeScript reflection host to:

* inspect / process ES private fields. e.g. `#someField` — those are
  ignored right now and we would want to check them to issue
  diagnostics.

* determine an access level of a class member. E.g. a member may be
  public, may be private, may be ES private, or public readonly. This
  can then be used in various checks later.

PR Close #54981
dylhunn pushed a commit that referenced this pull request Mar 27, 2024
…r APIs (#54981)

An initializer API like `input`, `output`, or signal queries may not be
compatible with certain access levels. E.g. queries cannot work with ES
private class fields.

This commit introduces a check for access levels into the initializer
API recognition— enforcing that every initializer API *clearly*
specifies what type of access is allowed.

PR Close #54981
dylhunn pushed a commit that referenced this pull request Mar 27, 2024
…ivate fields (#54981)

This commit ensures that the new APIs like `input`, `model`, `output`,
or signal-based queries are not accidentally used on fields that have a
problematic visibility/access level that won't work.

For example, queries defined using a private identifier (e.g. `#bla`)
will not be accessible by the Angular runtime and therefore _dont_ work.

This commit ensures:

- `input` is only declared via public and protected fields.
- `output` is only declared via public and protected fields.
- `model` is only declared via public and protected fields.
- signal queries are only declared via public, protected and TS private
  fields (`private` works, while `#bla` does not).

Fixes #54863.

PR Close #54981
dylhunn pushed a commit that referenced this pull request Mar 27, 2024
…nosticError` (#54981)

For `FatalDiagnosticError` we are currently hiding the `message` string
field in favor of the actual TS `diagnosticMessage`.

This works as expected, but makes these errors hard to debug in certain
environments (e.g. Jasmine). That is because `null` is the value of
`message` at runtime. We fix this by just overriding the type, like we
originally intended to do.

In addition, we properly render message chains in the `Error#message`
field— so that these errors, when uncaught, are somewhat reasonable and
can be useful.

PR Close #54981
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler PullApprove: disable 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