Skip to content

Commit 82682bb

Browse files
matskomatsko
committed
refactor(ivy): enable sanitization support for the new styling algorithm (angular#30667)
This patch is one of the final patches to refactor the styling algorithm to be more efficient, performant and less complex. This patch enables sanitization support for map-based and prop-based style bindings. PR Close angular#30667
1 parent d72479b commit 82682bb

23 files changed

+726
-103
lines changed

‎packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@
77
*/
88

99
import {AttributeMarker, ViewEncapsulation} from '@angular/compiler/src/core';
10+
import {CompilerStylingMode, compilerSetStylingMode} from '@angular/compiler/src/render3/view/styling_state';
1011
import {setup} from '@angular/compiler/test/aot/test_util';
12+
1113
import {compile, expectEmit} from './mock_compile';
1214

1315
describe('compiler compliance: styling', () => {
@@ -1463,4 +1465,122 @@ describe('compiler compliance: styling', () => {
14631465
const result = compile(files, angularFiles);
14641466
expectEmit(result.source, template, 'Incorrect template');
14651467
});
1468+
1469+
describe('new styling refactor', () => {
1470+
beforeEach(() => { compilerSetStylingMode(CompilerStylingMode.UseNew); });
1471+
1472+
afterEach(() => { compilerSetStylingMode(CompilerStylingMode.UseOld); });
1473+
1474+
it('should generate a `styleSanitizer` instruction when one or more sanitizable style properties are statically detected',
1475+
() => {
1476+
const files = {
1477+
app: {
1478+
'spec.ts': `
1479+
import {Component, NgModule} from '@angular/core';
1480+
1481+
@Component({
1482+
selector: 'my-app',
1483+
template: \`
1484+
<div [style.background-image]="bgExp"></div>
1485+
\`
1486+
})
1487+
export class MyAppComp {
1488+
bgExp = '';
1489+
}
1490+
`
1491+
}
1492+
};
1493+
1494+
const template = `
1495+
template: function MyAppComp_Template(rf, ctx) {
1496+
…
1497+
if (rf & 2) {
1498+
$r3$.ɵɵselect(0);
1499+
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
1500+
$r3$.ɵɵstyleProp(0, ctx.bgExp);
1501+
$r3$.ɵɵstylingApply();
1502+
}
1503+
…
1504+
}
1505+
`;
1506+
1507+
const result = compile(files, angularFiles);
1508+
expectEmit(result.source, template, 'Incorrect template');
1509+
});
1510+
1511+
it('should generate a `styleSanitizer` instruction when a `styleMap` instruction is used',
1512+
() => {
1513+
const files = {
1514+
app: {
1515+
'spec.ts': `
1516+
import {Component, NgModule} from '@angular/core';
1517+
1518+
@Component({
1519+
selector: 'my-app',
1520+
template: \`
1521+
<div [style]="mapExp"></div>
1522+
\`
1523+
})
1524+
export class MyAppComp {
1525+
mapExp = {};
1526+
}
1527+
`
1528+
}
1529+
};
1530+
1531+
const template = `
1532+
template: function MyAppComp_Template(rf, ctx) {
1533+
…
1534+
if (rf & 2) {
1535+
$r3$.ɵɵselect(0);
1536+
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
1537+
$r3$.ɵɵstyleMap(ctx.mapExp);
1538+
$r3$.ɵɵstylingApply();
1539+
}
1540+
…
1541+
}
1542+
`;
1543+
1544+
const result = compile(files, angularFiles);
1545+
expectEmit(result.source, template, 'Incorrect template');
1546+
});
1547+
1548+
it('shouldn\'t generate a `styleSanitizer` instruction when class-based instructions are used',
1549+
() => {
1550+
const files = {
1551+
app: {
1552+
'spec.ts': `
1553+
import {Component, NgModule} from '@angular/core';
1554+
1555+
@Component({
1556+
selector: 'my-app',
1557+
template: \`
1558+
<div [class]="mapExp" [class.name]="nameExp"></div>
1559+
\`
1560+
})
1561+
export class MyAppComp {
1562+
mapExp = {};
1563+
nameExp = true;
1564+
}
1565+
`
1566+
}
1567+
};
1568+
1569+
const template = `
1570+
template: function MyAppComp_Template(rf, ctx) {
1571+
…
1572+
if (rf & 2) {
1573+
$r3$.ɵɵselect(0);
1574+
$r3$.ɵɵclassMap(ctx.mapExp);
1575+
$r3$.ɵɵclassProp(0, ctx.nameExp);
1576+
$r3$.ɵɵstylingApply();
1577+
}
1578+
…
1579+
}
1580+
`;
1581+
1582+
const result = compile(files, angularFiles);
1583+
expectEmit(result.source, template, 'Incorrect template');
1584+
});
1585+
});
14661586
});

‎packages/compiler/src/render3/r3_identifiers.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ export class Identifiers {
8080

8181
static stylingApply: o.ExternalReference = {name: 'ɵɵstylingApply', moduleName: CORE};
8282

83+
static styleSanitizer: o.ExternalReference = {name: 'ɵɵstyleSanitizer', moduleName: CORE};
84+
8385
static elementHostAttrs: o.ExternalReference = {name: 'ɵɵelementHostAttrs', moduleName: CORE};
8486

8587
static containerCreate: o.ExternalReference = {name: 'ɵɵcontainer', moduleName: CORE};

‎packages/compiler/src/render3/view/styling_builder.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ export class StylingBuilder {
8989
/** an array of each [class.name] input */
9090
private _singleClassInputs: BoundStylingEntry[]|null = null;
9191
private _lastStylingInput: BoundStylingEntry|null = null;
92+
private _firstStylingInput: BoundStylingEntry|null = null;
9293

9394
// maps are used instead of hash maps because a Map will
9495
// retain the ordering of the keys
@@ -181,6 +182,7 @@ export class StylingBuilder {
181182
registerIntoMap(this._stylesIndex, property);
182183
}
183184
this._lastStylingInput = entry;
185+
this._firstStylingInput = this._firstStylingInput || entry;
184186
this.hasBindings = true;
185187
return entry;
186188
}
@@ -200,6 +202,7 @@ export class StylingBuilder {
200202
registerIntoMap(this._classesIndex, property);
201203
}
202204
this._lastStylingInput = entry;
205+
this._firstStylingInput = this._firstStylingInput || entry;
203206
this.hasBindings = true;
204207
return entry;
205208
}
@@ -453,13 +456,25 @@ export class StylingBuilder {
453456
};
454457
}
455458

459+
private _buildSanitizerFn() {
460+
return {
461+
sourceSpan: this._firstStylingInput ? this._firstStylingInput.sourceSpan : null,
462+
reference: R3.styleSanitizer,
463+
allocateBindingSlots: 0,
464+
buildParams: () => [o.importExpr(R3.defaultStyleSanitizer)]
465+
};
466+
}
467+
456468
/**
457469
* Constructs all instructions which contain the expressions that will be placed
458470
* into the update block of a template function or a directive hostBindings function.
459471
*/
460472
buildUpdateLevelInstructions(valueConverter: ValueConverter) {
461473
const instructions: Instruction[] = [];
462474
if (this.hasBindings) {
475+
if (compilerIsNewStylingInUse() && this._useDefaultSanitizer) {
476+
instructions.push(this._buildSanitizerFn());
477+
}
463478
const styleMapInstruction = this.buildStyleMapInstruction(valueConverter);
464479
if (styleMapInstruction) {
465480
instructions.push(styleMapInstruction);

‎packages/core/src/render3/debug.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,8 @@ export function toDebugNodes(tNode: TNode | null, lView: LView): DebugNode[]|nul
195195
let styles: DebugNewStyling|null = null;
196196
let classes: DebugNewStyling|null = null;
197197
if (runtimeIsNewStylingInUse()) {
198-
styles = tNode.newStyles ? new NodeStylingDebug(tNode.newStyles, lView) : null;
199-
classes = tNode.newClasses ? new NodeStylingDebug(tNode.newClasses, lView) : null;
198+
styles = tNode.newStyles ? new NodeStylingDebug(tNode.newStyles, lView, false) : null;
199+
classes = tNode.newClasses ? new NodeStylingDebug(tNode.newClasses, lView, true) : null;
200200
}
201201

202202
debugNodes.push({

‎packages/core/src/render3/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ export {
102102
ɵɵselect,
103103
ɵɵstyleMap,
104104
ɵɵstyleProp,
105+
ɵɵstyleSanitizer,
105106
ɵɵstyling,
106107
ɵɵstylingApply,
107108
ɵɵtemplate,

‎packages/core/src/render3/instructions/all.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,5 +45,6 @@ export * from './property';
4545
export * from './property_interpolation';
4646
export * from './select';
4747
export * from './styling';
48+
export {styleSanitizer as ɵɵstyleSanitizer} from '../styling_next/instructions';
4849
export * from './text';
4950
export * from './text_interpolation';

‎packages/core/src/render3/jit/environment.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ export const angularCoreEnv: {[name: string]: Function} =
121121
'ɵɵstyling': r3.ɵɵstyling,
122122
'ɵɵstyleMap': r3.ɵɵstyleMap,
123123
'ɵɵstyleProp': r3.ɵɵstyleProp,
124+
'ɵɵstyleSanitizer': r3.ɵɵstyleSanitizer,
124125
'ɵɵstylingApply': r3.ɵɵstylingApply,
125126
'ɵɵclassProp': r3.ɵɵclassProp,
126127
'ɵɵselect': r3.ɵɵselect,

‎packages/core/src/render3/styling/class_and_style_bindings.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* Use of this source code is governed by an MIT-style license that can be
66
* found in the LICENSE file at https://angular.io/license
77
*/
8-
import {StyleSanitizeFn} from '../../sanitization/style_sanitizer';
8+
import {StyleSanitizeFn, StyleSanitizeMode} from '../../sanitization/style_sanitizer';
99
import {EMPTY_ARRAY, EMPTY_OBJ} from '../empty';
1010
import {AttributeMarker, TAttributes} from '../interfaces/node';
1111
import {BindingStore, BindingType, Player, PlayerBuilder, PlayerFactory, PlayerIndex} from '../interfaces/player';
@@ -943,7 +943,9 @@ function updateSingleStylingValue(
943943
if (currDirective !== directiveIndex) {
944944
const prop = getProp(context, singleIndex);
945945
const sanitizer = getStyleSanitizer(context, directiveIndex);
946-
setSanitizeFlag(context, singleIndex, (sanitizer && sanitizer(prop)) ? true : false);
946+
setSanitizeFlag(
947+
context, singleIndex,
948+
(sanitizer && sanitizer(prop, null, StyleSanitizeMode.ValidateProperty)) ? true : false);
947949
}
948950

949951
// the value will always get updated (even if the dirty flag is skipped)
@@ -1141,7 +1143,8 @@ export function setStyle(
11411143
native: any, prop: string, value: string | null, renderer: Renderer3,
11421144
sanitizer: StyleSanitizeFn | null, store?: BindingStore | null,
11431145
playerBuilder?: ClassAndStylePlayerBuilder<any>| null) {
1144-
value = sanitizer && value ? sanitizer(prop, value) : value;
1146+
value =
1147+
sanitizer && value ? sanitizer(prop, value, StyleSanitizeMode.ValidateAndSanitize) : value;
11451148
if (store || playerBuilder) {
11461149
if (store) {
11471150
store.setValue(prop, value);
@@ -1461,7 +1464,9 @@ function valueExists(value: string | null | boolean, isClassBased?: boolean) {
14611464
function prepareInitialFlag(
14621465
context: StylingContext, prop: string, entryIsClassBased: boolean,
14631466
sanitizer?: StyleSanitizeFn | null) {
1464-
let flag = (sanitizer && sanitizer(prop)) ? StylingFlags.Sanitize : StylingFlags.None;
1467+
let flag = (sanitizer && sanitizer(prop, null, StyleSanitizeMode.ValidateProperty)) ?
1468+
StylingFlags.Sanitize :
1469+
StylingFlags.None;
14651470

14661471
let initialIndex: number;
14671472
if (entryIsClassBased) {

0 commit comments

Comments
 (0)