Skip to content

refactor: use lightweight token for injecting radio group#19645

Merged
devversion merged 2 commits intoangular:lightweight-token-optimizationsfrom
devversion:wip/light-weight-tokens-5
Jun 17, 2020
Merged

refactor: use lightweight token for injecting radio group#19645
devversion merged 2 commits intoangular:lightweight-token-optimizationsfrom
devversion:wip/light-weight-tokens-5

Conversation

@devversion
Copy link
Member

The Angular Material MatRadioButton component currently optionally
injects the MatRadioGroup. This causes the MatRadioGroup implementation
to be always retained in application bundles.

This is problematic as the use of radio groups is not always required.
i.e. it's valid to use a standalone radio button. In those cases, we do
not want to retain the radio group implementation unnecessarily.

This has always been an issue, but the issue became more significant
in Ivy where component factory and definitions are attached directly
to the MatRadioGroup class (resulting in a more significant size increase).

We fix this by using a lightweight token for injecting the parent
radio group optionally. This solves the retention issue if a standalone
radio button is used.

Related to: #19576.

@devversion devversion added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release labels Jun 15, 2020
@devversion devversion requested a review from a team June 15, 2020 13:54
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jun 15, 2020
* alternative token to the actual `MatRadioGroup` class which could cause unnecessary
* retention of the class and its component metadata.
*/
export const MAT_RADIO_GROUP =
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI: This is a separate token in the MDC-based implementation as the non-MDC implementation should not interfere with the MDC-based one.

@devversion devversion added the merge: preserve commits When the PR is merged, a rebase and merge should be performed label Jun 15, 2020
@devversion devversion force-pushed the wip/light-weight-tokens-5 branch from b75321f to 7716828 Compare June 15, 2020 13:57
Copy link
Contributor

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added lgtm action: merge The PR is ready for merge by the caretaker labels Jun 15, 2020
@devversion devversion added P2 The issue is important to a large percentage of users, with a workaround and removed P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent labels Jun 16, 2020
The Angular Material `MatRadioButton` component currently
optionally injects the `MatRadioGroup`. This causes the
`MatRadioGroup` implementation to be always retained in
application bundles.

This is problematic as the use of radio groups is not
always required. i.e. it's valid to use a standalone
radio button. In those cases, we do not want to retain
the radio group implementation unnecessarily.

This has always been an issue, but the issue became
more significant in Ivy where component factory and definitions
are attached directly to the `MatRadioGroup` class (resulting
in a more significant size increase).

We fix this by using a lightweight token for injecting the parent
radio group optionally. This solves the retention issue if a standalone
radio button is used.

Related to: angular#19576.
@devversion devversion force-pushed the wip/light-weight-tokens-5 branch from 7716828 to 2c25515 Compare June 16, 2020 19:35
@devversion devversion changed the base branch from master to lightweight-token-optimizations June 17, 2020 17:05
@devversion devversion merged commit f30d9e4 into angular:lightweight-token-optimizations Jun 17, 2020
@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 Jul 18, 2020
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 cla: yes PR author has agreed to Google's Contributor License Agreement merge: preserve commits When the PR is merged, a rebase and merge should be performed P2 The issue is important to a large percentage of users, with a workaround target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants