Follow-up cleanups for model inputs by crisbeto · Pull Request #54387 · angular/angular

crisbeto · 2024-02-12T10:03:36Z

Includes some cleanups around model() inputs as well as more test coverage. See the individual commits for more info.

crisbeto · 2024-02-12T10:15:22Z

packages/core/src/render3/instructions/listener.ts

We could simplify it a bit by tracking the Subscription.unsubscribe here, but then we run the risk of either the this not being correct when it's invoked, or having to track the context of the function, or having to create a new instance using .bind.

devversion

LGTM. Thx for all the tests! I realize I also should add one for references_and_rename. All others we have

devversion · 2024-02-12T11:13:59Z

packages/core/src/render3/instructions/listener.ts

Nice! need this for output as well. Set this up as a preparation and to be able to "implement" the interface

packages/core/src/render3/node_manipulation.ts

alxhub

Reviewed-for: public-api

alxhub

Reviewed-for: public-api, language-service

Reworks `ModelSignal.subscribe` so it doesn't have to wrap its value to look like a subscription.

Reworks the model so that it reuses `INPUT_SIGNAL_NODE` instead of implementing its own.

Getting the typing for `ɵunwrapWritableSignal` just right was tricky so these changes add some tests to ensure that we don't regress. Also reworks the type tester a bit to make it easier to find where to add new test files.

Sets up type checking diagnostic tests for model() inputs.

Splits up the tests for `input()` and `model()` into separate files.

Updates the language service tests to cover `model()` inputs.

AndrewKushnir

Reviewed-for: public-api

jessicajaniuk

reviewed-for: public-api

AndrewKushnir · 2024-02-12T19:01:57Z

This PR was merged into the repository by commit 0b955b8.

Reworks `ModelSignal.subscribe` so it doesn't have to wrap its value to look like a subscription. PR Close #54387

Reworks the model so that it reuses `INPUT_SIGNAL_NODE` instead of implementing its own. PR Close #54387

Getting the typing for `ɵunwrapWritableSignal` just right was tricky so these changes add some tests to ensure that we don't regress. Also reworks the type tester a bit to make it easier to find where to add new test files. PR Close #54387

Sets up type checking diagnostic tests for model() inputs. PR Close #54387

Splits up the tests for `input()` and `model()` into separate files. PR Close #54387

Updates the language service tests to cover `model()` inputs. PR Close #54387

Reworks the model so that it reuses `INPUT_SIGNAL_NODE` instead of implementing its own. PR Close #54387

Getting the typing for `ɵunwrapWritableSignal` just right was tricky so these changes add some tests to ensure that we don't regress. Also reworks the type tester a bit to make it easier to find where to add new test files. PR Close #54387

Sets up type checking diagnostic tests for model() inputs. PR Close #54387

Splits up the tests for `input()` and `model()` into separate files. PR Close #54387

Updates the language service tests to cover `model()` inputs. PR Close #54387

…54387) Reworks `ModelSignal.subscribe` so it doesn't have to wrap its value to look like a subscription. PR Close angular#54387

Reworks the model so that it reuses `INPUT_SIGNAL_NODE` instead of implementing its own. PR Close angular#54387

Getting the typing for `ɵunwrapWritableSignal` just right was tricky so these changes add some tests to ensure that we don't regress. Also reworks the type tester a bit to make it easier to find where to add new test files. PR Close angular#54387

Sets up type checking diagnostic tests for model() inputs. PR Close angular#54387

Splits up the tests for `input()` and `model()` into separate files. PR Close angular#54387

Updates the language service tests to cover `model()` inputs. PR Close angular#54387

angular-automatic-lock-bot · 2024-03-14T00:08:19Z

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.}

crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer area: compiler Issues related to `ngc`, Angular's template compiler target: rc This PR is targeted for the next release-candidate labels Feb 12, 2024

crisbeto added this to the v17.2 Candidates milestone Feb 12, 2024

crisbeto force-pushed the model-input-followup branch from 627baa3 to 67ce676 Compare February 12, 2024 10:08

crisbeto commented Feb 12, 2024

View reviewed changes

crisbeto force-pushed the model-input-followup branch from 67ce676 to 77c8173 Compare February 12, 2024 10:31

crisbeto requested a review from devversion February 12, 2024 10:40

crisbeto marked this pull request as ready for review February 12, 2024 10:40

pullapprove bot requested a review from atscott February 12, 2024 10:40

devversion approved these changes Feb 12, 2024

View reviewed changes

alxhub approved these changes Feb 12, 2024

View reviewed changes

pullapprove bot requested review from AndrewKushnir and jessicajaniuk February 12, 2024 15:40

crisbeto added 6 commits February 12, 2024 17:34

refactor(core): avoid wrapper around subscribe return value

5a7bcdd

Reworks `ModelSignal.subscribe` so it doesn't have to wrap its value to look like a subscription.

refactor(core): reuse input signal node for models

c666b5d

Reworks the model so that it reuses `INPUT_SIGNAL_NODE` instead of implementing its own.

test(core): add type tests for ɵunwrapWritableSignal

f98707d

Getting the typing for `ɵunwrapWritableSignal` just right was tricky so these changes add some tests to ensure that we don't regress. Also reworks the type tester a bit to make it easier to find where to add new test files.

refactor(compiler-cli): add diagnostic tests for models

b66420c

Sets up type checking diagnostic tests for model() inputs.

refactor(compiler-cli): split input and model tests

98ddb75

Splits up the tests for `input()` and `model()` into separate files.

test(language-service): add tests for model inputs

0eac0eb

Updates the language service tests to cover `model()` inputs.

crisbeto force-pushed the model-input-followup branch from 77c8173 to 0eac0eb Compare February 12, 2024 16:34

AndrewKushnir approved these changes Feb 12, 2024

View reviewed changes

jessicajaniuk approved these changes Feb 12, 2024

View reviewed changes

crisbeto removed the request for review from atscott February 12, 2024 17:55

crisbeto added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Feb 12, 2024

AndrewKushnir pushed a commit that referenced this pull request Feb 12, 2024

refactor(core): avoid wrapper around subscribe return value (#54387)

453e00c

Reworks `ModelSignal.subscribe` so it doesn't have to wrap its value to look like a subscription. PR Close #54387

AndrewKushnir pushed a commit that referenced this pull request Feb 12, 2024

refactor(core): reuse input signal node for models (#54387)

0ecc2d1

Reworks the model so that it reuses `INPUT_SIGNAL_NODE` instead of implementing its own. PR Close #54387

AndrewKushnir pushed a commit that referenced this pull request Feb 12, 2024

refactor(compiler-cli): add diagnostic tests for models (#54387)

2363e77

Sets up type checking diagnostic tests for model() inputs. PR Close #54387

AndrewKushnir pushed a commit that referenced this pull request Feb 12, 2024

refactor(compiler-cli): split input and model tests (#54387)

369e96c

Splits up the tests for `input()` and `model()` into separate files. PR Close #54387

AndrewKushnir pushed a commit that referenced this pull request Feb 12, 2024

test(language-service): add tests for model inputs (#54387)

ef965ac

Updates the language service tests to cover `model()` inputs. PR Close #54387

AndrewKushnir closed this in 4a7ca50 Feb 12, 2024

AndrewKushnir pushed a commit that referenced this pull request Feb 12, 2024

refactor(core): reuse input signal node for models (#54387)

58666e9

Reworks the model so that it reuses `INPUT_SIGNAL_NODE` instead of implementing its own. PR Close #54387

AndrewKushnir pushed a commit that referenced this pull request Feb 12, 2024

refactor(compiler-cli): add diagnostic tests for models (#54387)

b9ba136

Sets up type checking diagnostic tests for model() inputs. PR Close #54387

AndrewKushnir pushed a commit that referenced this pull request Feb 12, 2024

refactor(compiler-cli): split input and model tests (#54387)

6897b76

Splits up the tests for `input()` and `model()` into separate files. PR Close #54387

AndrewKushnir pushed a commit that referenced this pull request Feb 12, 2024

test(language-service): add tests for model inputs (#54387)

0b955b8

Updates the language service tests to cover `model()` inputs. PR Close #54387

atscott pushed a commit to atscott/angular that referenced this pull request Feb 16, 2024

refactor(core): reuse input signal node for models (angular#54387)

623f03f

Reworks the model so that it reuses `INPUT_SIGNAL_NODE` instead of implementing its own. PR Close angular#54387

atscott pushed a commit to atscott/angular that referenced this pull request Feb 16, 2024

refactor(compiler-cli): add diagnostic tests for models (angular#54387)

7744430

Sets up type checking diagnostic tests for model() inputs. PR Close angular#54387

atscott pushed a commit to atscott/angular that referenced this pull request Feb 16, 2024

refactor(compiler-cli): split input and model tests (angular#54387)

7071a25

Splits up the tests for `input()` and `model()` into separate files. PR Close angular#54387

atscott pushed a commit to atscott/angular that referenced this pull request Feb 16, 2024

test(language-service): add tests for model inputs (angular#54387)

14833cc

Updates the language service tests to cover `model()` inputs. PR Close angular#54387

angular-automatic-lock-bot bot locked and limited conversation to collaborators Mar 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Follow-up cleanups for model inputs#54387

Follow-up cleanups for model inputs#54387
crisbeto wants to merge 6 commits intoangular:mainfrom
crisbeto:model-input-followup

crisbeto commented Feb 12, 2024 •

edited

Loading

Uh oh!

crisbeto Feb 12, 2024

Uh oh!

devversion left a comment •

edited

Loading

Uh oh!

devversion Feb 12, 2024

Uh oh!

Uh oh!

alxhub left a comment

Uh oh!

alxhub left a comment

Uh oh!

AndrewKushnir left a comment

Uh oh!

jessicajaniuk left a comment

Uh oh!

AndrewKushnir commented Feb 12, 2024

Uh oh!

angular-automatic-lock-bot bot commented Mar 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

crisbeto commented Feb 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

crisbeto Feb 12, 2024

Choose a reason for hiding this comment

Uh oh!

devversion left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

devversion Feb 12, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alxhub left a comment

Choose a reason for hiding this comment

Uh oh!

alxhub left a comment

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir left a comment

Choose a reason for hiding this comment

Uh oh!

jessicajaniuk left a comment

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir commented Feb 12, 2024

Uh oh!

angular-automatic-lock-bot bot commented Mar 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

crisbeto commented Feb 12, 2024 •

edited

Loading

devversion left a comment •

edited

Loading