Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch the CDK to input transforms #27778

Merged
merged 11 commits into from
Sep 12, 2023
Merged

Conversation

crisbeto
Copy link
Member

Reworks most of the places where we use input coercion in the CDK to input transforms.

Switches inputs in cdk/a11y to use transforms instead of getters/setters.
Switches inputs in cdk/accordion to use transforms instead of getters/setters.
Switches inputs in cdk/drag-drop to use transforms instead of getters/setters.
Switches inputs in cdk/listbox to use transforms instead of getters/setters.
@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: major This PR is targeted for the next major release merge: preserve commits When the PR is merged, a rebase and merge should be performed labels Sep 10, 2023
@crisbeto crisbeto changed the title Switch the CDK input transforms Switch the CDK to input transforms Sep 10, 2023
@crisbeto crisbeto force-pushed the cdk-input-transforms branch from 36dc52c to 44e6460 Compare September 10, 2023 17:11
Switches inputs in cdk/menu to use transforms instead of getters/setters.
Switches inputs in cdk/observers to use transforms instead of getters/setters.
Switches inputs in cdk/a11y to use transforms instead of getters/setters.
Switches inputs in cdk/scrolling to use transforms instead of getters/setters.
Switches inputs in cdk/stepper to use transforms instead of getters/setters.
Switches inputs in cdk/table to use transforms instead of getters/setters.
Switches inputs in cdk/text-field to use transforms instead of getters/setters.
@crisbeto crisbeto force-pushed the cdk-input-transforms branch from 44e6460 to 83c3fa8 Compare September 10, 2023 17:17
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM but I think we should reach consensus on https://angular-team.slack.com/archives/C040TF8UT/p1694087146008889 sooner than later- because we are clearly opening up the types to anything with a PR like this

@andrewseguin
Copy link
Contributor

+1 @devversion Would be good to get this clarified before the release

Also - we should mark the current coercion functions as deprecated so that we can remove them in v19

Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

@crisbeto
Copy link
Member Author

We discussed this today during standup. I'll merge this PR and work on making the typings stronger on the FW side before v17.

@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label Sep 12, 2023
@crisbeto crisbeto merged commit a1fb9f9 into angular:main Sep 12, 2023
@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 Oct 13, 2023
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 merge: preserve commits When the PR is merged, a rebase and merge should be performed P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants