Skip to content

refactor: use lightweight token for chip avatar, remove and trailing icon#19693

Merged
mmalerba merged 2 commits intoangular:lightweight-token-optimizationsfrom
devversion:wip/light-weight-tokens-10
Jun 19, 2020
Merged

refactor: use lightweight token for chip avatar, remove and trailing icon#19693
mmalerba merged 2 commits intoangular:lightweight-token-optimizationsfrom
devversion:wip/light-weight-tokens-10

Conversation

@devversion
Copy link
Member

@devversion devversion commented Jun 18, 2020

Currently MatChip always causes MatChipAvatar, MatChipRemove and
MatChipTrailingIcon to be retained. These are optional parts of a chip and
shouldn't be retained always. The retention of these classes wasn't noticeable
in View Engine as no factories were generated for directives. With Ivy though,
factories and definitions are generated for these directives and a size increase
is noticeable.

We should use lightweight tokens for querying of those directives. These tokens
can be considered a reference to the actual directives. That allows us to query
for these directives in content queries, or DI without the risk of retaining the
full classes w/ factories/definitions attached.

That helps with the case where not all of the optional chip parts are used. If all
parts are used, the app will slightly increase due to the provided injection tokens,
but that is of ~150b, while the savings are of ~800b in the common case. The size
benefit is even more noticeable in the MDC-based implementation of chips
as the directives contain a lot more logic w/ MDC foundations.

Related to: #19576

…icon

Currently `MatChip` always causes `MatChipAvatar`, `MatChipRemove` and
`MatChipTrailingIcon` to be retained. These are optional parts of a
chip and shouldn't be retained always. The retention of these classes
wasn't noticeable in View Engine as no factories were generated for
directives. With Ivy though, factories and definitions are generated
for these directives and a size increase is noticeable.

We should use lightweight tokens for querying of those directives.
That will improve tree-shaking for the case where not all of these
parts are used. If all parts are used, the app will slightly increase
due to the provided injection tokens, but that is of average 150b, while
the savings are of ~800b in the common case.

Related to: angular#19576
@devversion devversion added P2 The issue is important to a large percentage of users, with a workaround merge safe labels Jun 18, 2020
@devversion devversion requested a review from a team June 18, 2020 20:31
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jun 18, 2020
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

LGTM

@mmalerba mmalerba added lgtm action: merge The PR is ready for merge by the caretaker labels Jun 18, 2020
@mmalerba mmalerba merged commit 84e07bd into angular:lightweight-token-optimizations Jun 19, 2020
devversion added a commit to devversion/material2 that referenced this pull request Jun 22, 2020
…icon (angular#19693)

* build: add size test for basic chip-list with chip

* refactor: use lightweight token for chip avatar, remove and trailing icon

Currently `MatChip` always causes `MatChipAvatar`, `MatChipRemove` and
`MatChipTrailingIcon` to be retained. These are optional parts of a
chip and shouldn't be retained always. The retention of these classes
wasn't noticeable in View Engine as no factories were generated for
directives. With Ivy though, factories and definitions are generated
for these directives and a size increase is noticeable.

We should use lightweight tokens for querying of those directives.
That will improve tree-shaking for the case where not all of these
parts are used. If all parts are used, the app will slightly increase
due to the provided injection tokens, but that is of average 150b, while
the savings are of ~800b in the common case.

Related to: angular#19576
devversion added a commit that referenced this pull request Jun 22, 2020
…icon (#19693)

* build: add size test for basic chip-list with chip

* refactor: use lightweight token for chip avatar, remove and trailing icon

Currently `MatChip` always causes `MatChipAvatar`, `MatChipRemove` and
`MatChipTrailingIcon` to be retained. These are optional parts of a
chip and shouldn't be retained always. The retention of these classes
wasn't noticeable in View Engine as no factories were generated for
directives. With Ivy though, factories and definitions are generated
for these directives and a size increase is noticeable.

We should use lightweight tokens for querying of those directives.
That will improve tree-shaking for the case where not all of these
parts are used. If all parts are used, the app will slightly increase
due to the provided injection tokens, but that is of average 150b, while
the savings are of ~800b in the common case.

Related to: #19576
devversion added a commit to devversion/material2 that referenced this pull request Jun 23, 2020
…icon (angular#19693)

* build: add size test for basic chip-list with chip

* refactor: use lightweight token for chip avatar, remove and trailing icon

Currently `MatChip` always causes `MatChipAvatar`, `MatChipRemove` and
`MatChipTrailingIcon` to be retained. These are optional parts of a
chip and shouldn't be retained always. The retention of these classes
wasn't noticeable in View Engine as no factories were generated for
directives. With Ivy though, factories and definitions are generated
for these directives and a size increase is noticeable.

We should use lightweight tokens for querying of those directives.
That will improve tree-shaking for the case where not all of these
parts are used. If all parts are used, the app will slightly increase
due to the provided injection tokens, but that is of average 150b, while
the savings are of ~800b in the common case.

Related to: angular#19576
jelbourn pushed a commit that referenced this pull request Jun 29, 2020
…icon (#19693)

* build: add size test for basic chip-list with chip

* refactor: use lightweight token for chip avatar, remove and trailing icon

Currently `MatChip` always causes `MatChipAvatar`, `MatChipRemove` and
`MatChipTrailingIcon` to be retained. These are optional parts of a
chip and shouldn't be retained always. The retention of these classes
wasn't noticeable in View Engine as no factories were generated for
directives. With Ivy though, factories and definitions are generated
for these directives and a size increase is noticeable.

We should use lightweight tokens for querying of those directives.
That will improve tree-shaking for the case where not all of these
parts are used. If all parts are used, the app will slightly increase
due to the provided injection tokens, but that is of average 150b, while
the savings are of ~800b in the common case.

Related to: #19576
@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 20, 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 P2 The issue is important to a large percentage of users, with a workaround

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants