fix(upgrade): support input signal bindings #57020
Conversation
pkozlowski-opensource
left a comment
There was a problem hiding this comment.
LGTM
Reviewed-for: public-api
Reviewed-for: fw-core
|
Adding the cleanup label as G3 is upset about dependencies (see presubmit) |
86497ef to
dcc063a
Compare
dcc063a to
0360565
Compare
`@angular/upgrade` writes to inputs when downgrading an Angular 2+ component into an Angular.JS adapter. Previously, it wrote directly to the input property, which isn't compatible with input signals. It also handles `ngOnChanges` directly. The correct way to support input signals would be to refactor upgrade to use `ComponentRef.setInput`, which also handles `ngOnChanges` internally. However, this refactoring might be more breaking since it would change the timing of certain operations. Instead, this commit updates the code to recognize `InputSignal` and write it through the `InputSignalNode`. This avoids the above breaking changes for now, until a bigger refactoring can be tested. Fixes angular#56860.
0360565 to
3de9674
Compare
|
I see that you just added the If you want your PR to be merged, it has to pass all the CI checks. If you can't get the PR to a green state due to flakes or broken |
|
Caretaker (that's me, talking to myself here): this change is passing in g3 - there was 1 failing test which was fixed on the upstream end |
|
This PR was merged into the repository by commit 5f56a65. The changes were merged into the following branches: main, 18.2.x |
`@angular/upgrade` writes to inputs when downgrading an Angular 2+ component into an Angular.JS adapter. Previously, it wrote directly to the input property, which isn't compatible with input signals. It also handles `ngOnChanges` directly. The correct way to support input signals would be to refactor upgrade to use `ComponentRef.setInput`, which also handles `ngOnChanges` internally. However, this refactoring might be more breaking since it would change the timing of certain operations. Instead, this commit updates the code to recognize `InputSignal` and write it through the `InputSignalNode`. This avoids the above breaking changes for now, until a bigger refactoring can be tested. Fixes #56860. PR Close #57020
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
@angular/upgradewrites to inputs when downgrading an Angular 2+ component into an Angular.JS adapter. Previously, it wrote directly to the input property, which isn't compatible with input signals. It also handlesngOnChangesdirectly.The correct way to support input signals would be to refactor upgrade to use
ComponentRef.setInput, which also handlesngOnChangesinternally. However, this refactoring might be more breaking since it would change the timing of certain operations. Instead, this commit updates the code to recognizeInputSignaland write it through theInputSignalNode. This avoids the above breaking changes for now, until a bigger refactoring can be tested.Fixes #56860.