Skip to content

[17.3.x]: Patch port of new signal API access modifier enforcement#55070

Closed
devversion wants to merge 6 commits intoangular:17.3.xfrom
devversion:patch-port-initializer
Closed

[17.3.x]: Patch port of new signal API access modifier enforcement#55070
devversion wants to merge 6 commits intoangular:17.3.xfrom
devversion:patch-port-initializer

Conversation

@devversion
Copy link
Member

Patch port of #54981

crisbeto and others added 6 commits March 27, 2024 18:20
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 added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release PullApprove: disable labels Mar 27, 2024
@devversion devversion requested a review from crisbeto March 27, 2024 18:21
@devversion devversion marked this pull request as ready for review March 27, 2024 18:49
@dylhunn
Copy link
Contributor

dylhunn commented Mar 28, 2024

This PR was merged into the repository by commit 5b67c94.

dylhunn pushed a commit that referenced this pull request Mar 28, 2024
…55070)

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.

PR Close #55070
dylhunn pushed a commit that referenced this pull request Mar 28, 2024
…nd visibility (#55070)

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 #55070
dylhunn pushed a commit that referenced this pull request Mar 28, 2024
…r APIs (#55070)

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 #55070
@dylhunn dylhunn closed this Mar 28, 2024
dylhunn pushed a commit that referenced this pull request Mar 28, 2024
…ivate fields (#55070)

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 #55070
dylhunn pushed a commit that referenced this pull request Mar 28, 2024
…nosticError` (#55070)

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 #55070
@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 28, 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 PullApprove: disable target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants