refactor(core): unify styling data format in attributes#39998
refactor(core): unify styling data format in attributes#39998AndrewKushnir wants to merge 1 commit intoangular:mainfrom
Conversation
There was a problem hiding this comment.
Is the isProjectionMode not important information?
What was the scenario where the class attribute existed but it was not in projection mode?
|
Quick update after discussion with @petebacondarwin: this work is a pre-requisite to the large refactoring to avoid generating duplicate attributes for inline templates (there is a portion of attributes that can be shared between "template" and "element" nodes). While this change seems to be useful as is, we'd like to create a followup PR that would implement actual deduplication, so that we can perform additional measurements to estimate the improvement to the payload size (and check if there are any runtime perf concerns). Keeping this PR in a draft state for now. Thank you. |
b743895 to
46aa24f
Compare
Currently styling-related attributes (`class` and `style`) are represented differently depending on the node type.
Given the following template:
```
<div *ngIf="exp" title="abc" class="a b c" style="margin: 10px"></div>
```
For element node (that represent a `<div>`), this information is present in the parsed format (separated by
`AttributeMarker`s), such as:
```
["title", "abc", ${AttrMarker.Classes}, "a", "b", "c", ${AttrMarker.Styles}, "margin", "10px"]
```
When for the template node (that represents desugared version of structural directive, such as
`<ng-template [ngIf]>`), the class and style information would be present in the raw format:
```
["title", "abc", "class", "a b c", "style", "margin: 10px", ${AttrMarker.Template}, "ngIf"]
```
The same applies to the `<ng-content class="a b c">` tags (raw class information is present there).
Such inconsistency requires extra logic at runtime to perform matching and also blocks potential future work to
deduplicate attributes shared between element nodes and inline template nodes.
This commit updates the logic to represent class and style attrs info consistently (in parsed format) for all node
types.
169908a to
8ccec57
Compare
|
@AndrewKushnir it looks like the work on this PR stalled a bit so I'm going to close it for now. Feel free to re-open when you are ready to restart work on it. |
|
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. |
Currently styling-related attributes (
classandstyle) are represented differently depending on the node type.Given the following template:
For element node (that represent a
<div>), this information is present in the parsed format (separated byAttributeMarkers), such as:When for the template node (that represents desugared version of structural directive, such as
<ng-template [ngIf]>),the class and style information would be present in the raw format:
The same applies to the
<ng-content class="a b c">tags (raw class information is present there).Such inconsistency requires extra logic at runtime to perform matching and also blocks potential future work to
deduplicate attributes shared between element nodes and inline template nodes.
This commit updates the logic to represent class and style attrs info consistently (in parsed format) for all node
types.
PR Type
What kind of change does this PR introduce?
Does this PR introduce a breaking change?