Skip to content

Commit

Permalink
revert: refactor(ivy): remove styleSanitizer instruction in favor of …
Browse files Browse the repository at this point in the history 
…an inline param (angular#34480) (angular#34910)

This reverts commit 84d24c0.

PR Close angular#34910
  • Loading branch information
@matsko
matsko committed Jan 22, 2020
1 parent 61ad50d commit 32489c7
Show file tree
Hide file tree
Showing 11 changed files with 135 additions and 128 deletions.
82 changes: 28 additions & 54 deletions packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelement(0, "div");
}
if (rf & 2) {
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleMap($ctx$.myStyleExp);
}
}
Expand Down Expand Up @@ -510,6 +511,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelement(0, "div", 0);
}
if (rf & 2) {
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleMap($ctx$.myStyleExp);
$r3$.ɵɵstyleProp("width", $ctx$.myWidth)("height", $ctx$.myHeight);
$r3$.ɵɵattribute("style", "border-width: 10px", $r3$.ɵɵsanitizeStyle);
Expand Down Expand Up @@ -555,7 +557,8 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelement(0, "div");
}
if (rf & 2) {
$r3$.ɵɵstyleProp("background-image", ctx.myImage, $r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleProp("background-image", ctx.myImage);
}
},
encapsulation: 2
Expand Down Expand Up @@ -813,6 +816,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelement(0, "div");
}
if (rf & 2) {
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleMap($ctx$.myStyleExp);
$r3$.ɵɵclassMap($ctx$.myClassExp);
}
Expand Down Expand Up @@ -854,6 +858,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleMap($r3$.ɵɵpipeBind1(1, 4, $ctx$.myStyleExp));
$r3$.ɵɵclassMap($r3$.ɵɵpipeBind1(2, 6, $ctx$.myClassExp));
}
Expand Down Expand Up @@ -906,6 +911,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleMap($r3$.ɵɵpipeBind2(1, 8, $ctx$.myStyleExp, 1000));
$r3$.ɵɵclassMap($r3$.ɵɵpureFunction0(20, _c0));
$r3$.ɵɵstyleProp("bar", $r3$.ɵɵpipeBind2(2, 11, $ctx$.barExp, 3000))("baz", $r3$.ɵɵpipeBind2(3, 14, $ctx$.bazExp, 4000));
Expand Down Expand Up @@ -1011,6 +1017,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelementHostAttrs($e0_attrs$);
}
if (rf & 2) {
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleMap(ctx.myStyle);
$r3$.ɵɵclassMap(ctx.myClass);
$r3$.ɵɵstyleProp("color", ctx.myColorProp);
Expand Down Expand Up @@ -1068,6 +1075,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵallocHostVars(8);
}
if (rf & 2) {
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleMap(ctx.myStyle);
$r3$.ɵɵclassMap(ctx.myClasses);
$r3$.ɵɵstyleProp("height", ctx.myHeightProp, "pt")("width", ctx.myWidthProp);
Expand Down Expand Up @@ -1125,6 +1133,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵelement(0, "div");
}
if (rf & 2) {
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleMap(ctx.myStyleExp);
$r3$.ɵɵclassMap(ctx.myClassExp);
$r3$.ɵɵstyleProp("height", ctx.myHeightExp);
Expand All @@ -1139,6 +1148,7 @@ describe('compiler compliance: styling', () => {
$r3$.ɵɵallocHostVars(6);
}
if (rf & 2) {
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleMap(ctx.myStyleExp);
$r3$.ɵɵclassMap(ctx.myClassExp);
$r3$.ɵɵstyleProp("width", ctx.myWidthExp);
Expand Down Expand Up @@ -1414,46 +1424,6 @@ describe('compiler compliance: styling', () => {
expectEmit(result.source, template, 'Incorrect handling of interpolated style properties');
});

it('should generate update instructions for interpolated style properties with a sanitizer',
() => {
const files = {
app: {
'spec.ts': `
import {Component} from '@angular/core';
@Component({
template: \`
<div style.background="url({{ myUrl1 }})"
style.borderImage="url({{ myUrl2 }}) {{ myRepeat }} auto"
style.boxShadow="{{ myBoxX }} {{ myBoxY }} {{ myBoxWidth }} black"></div>
\`
})
export class MyComponent {
myUrl1 = '...';
myUrl2 = '...';
myBoxX = '0px';
myBoxY = '0px';
myBoxWidth = '100px';
myRepeat = 'no-repeat';
}
`
}
};

const template = `
…
if (rf & 2) {
$r3$.ɵɵstylePropInterpolate1("background", "url(", ctx.myUrl1, ")", $r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstylePropInterpolate2("border-image", "url(", ctx.myUrl2, ") ", ctx.myRepeat, " auto", $r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstylePropInterpolate3("box-shadow", "", ctx.myBoxX, " ", ctx.myBoxY, " ", ctx.myBoxWidth, " black");
}
…
`;
const result = compile(files, angularFiles);

expectEmit(result.source, template, 'Incorrect handling of interpolated style properties');
});

it('should generate update instructions for interpolated style properties with !important',
() => {
const files = {
Expand Down Expand Up @@ -1879,6 +1849,7 @@ describe('compiler compliance: styling', () => {
}
if (rf & 2) {
$r3$.ɵɵhostProperty("id", ctx.id)("title", ctx.title);
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleMap(ctx.myStyle);
$r3$.ɵɵclassMap(ctx.myClass);
}
Expand Down Expand Up @@ -1931,7 +1902,7 @@ describe('compiler compliance: styling', () => {
});

describe('new styling refactor', () => {
it('should generate a sanitizer value into the instruction when one or more sanitizable style properties are statically detected',
it('should generate a `styleSanitizer` instruction when one or more sanitizable style properties are statically detected',
() => {
const files = {
app: {
Expand All @@ -1955,7 +1926,8 @@ describe('compiler compliance: styling', () => {
template: function MyAppComp_Template(rf, ctx) {
…
if (rf & 2) {
$r3$.ɵɵstyleProp("background-image", ctx.bgExp, $r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleProp("background-image", ctx.bgExp);
}
…
}
Expand All @@ -1965,10 +1937,11 @@ describe('compiler compliance: styling', () => {
expectEmit(result.source, template, 'Incorrect template');
});

it('should not add a sanitizer param when a `styleMap` instruction is used', () => {
const files = {
app: {
'spec.ts': `
it('should generate a `styleSanitizer` instruction when a `styleMap` instruction is used',
() => {
const files = {
app: {
'spec.ts': `
import {Component, NgModule} from '@angular/core';
@Component({
Expand All @@ -1981,24 +1954,25 @@ describe('compiler compliance: styling', () => {
mapExp = {};
}
`
}
};
}
};

const template = `
const template = `
template: function MyAppComp_Template(rf, ctx) {
…
if (rf & 2) {
$r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer);
$r3$.ɵɵstyleMap(ctx.mapExp);
}
…
}
`;

const result = compile(files, angularFiles);
expectEmit(result.source, template, 'Incorrect template');
});
const result = compile(files, angularFiles);
expectEmit(result.source, template, 'Incorrect template');
});

it('shouldn\'t generate a sanitizer param into the styling instruction when class-based instructions are used',
it('shouldn\'t generate a `styleSanitizer` instruction when class-based instructions are used',
() => {
const files = {
app: {
Expand Down
2 changes: 2 additions & 0 deletions packages/compiler/src/render3/r3_identifiers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ export class Identifiers {
static stylePropInterpolateV:
o.ExternalReference = {name: 'ɵɵstylePropInterpolateV', moduleName: CORE};

static styleSanitizer: o.ExternalReference = {name: 'ɵɵstyleSanitizer', moduleName: CORE};

static elementHostAttrs: o.ExternalReference = {name: 'ɵɵelementHostAttrs', moduleName: CORE};

static containerCreate: o.ExternalReference = {name: 'ɵɵcontainer', moduleName: CORE};
Expand Down
53 changes: 32 additions & 21 deletions packages/compiler/src/render3/view/styling_builder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ interface BoundStylingEntry {
name: string|null;
unit: string|null;
sourceSpan: ParseSourceSpan;
sanitize: boolean;
value: AST;
}

Expand Down Expand Up @@ -117,6 +116,10 @@ export class StylingBuilder {
private _initialStyleValues: string[] = [];
private _initialClassValues: string[] = [];

// certain style properties ALWAYS need sanitization
// this is checked each time new styles are encountered
private _useDefaultSanitizer = false;

constructor(private _elementIndexExpr: o.Expression, private _directiveExpr: o.Expression|null) {}

/**
Expand Down Expand Up @@ -176,13 +179,14 @@ export class StylingBuilder {
const {property, hasOverrideFlag, unit: bindingUnit} = parseProperty(name);
const entry: BoundStylingEntry = {
name: property,
sanitize: property ? isStyleSanitizable(property) : true,
unit: unit || bindingUnit, value, sourceSpan, hasOverrideFlag
};
if (isMapBased) {
this._useDefaultSanitizer = true;
this._styleMapInput = entry;
} else {
(this._singleStyleInputs = this._singleStyleInputs || []).push(entry);
this._useDefaultSanitizer = this._useDefaultSanitizer || isStyleSanitizable(name);
registerIntoMap(this._stylesIndex, property);
}
this._lastStylingInput = entry;
Expand All @@ -198,8 +202,8 @@ export class StylingBuilder {
return null;
}
const {property, hasOverrideFlag} = parseProperty(name);
const entry: BoundStylingEntry =
{name: property, value, sourceSpan, sanitize: false, hasOverrideFlag, unit: null};
const entry:
BoundStylingEntry = {name: property, value, sourceSpan, hasOverrideFlag, unit: null};
if (isMapBased) {
if (this._classMapInput) {
throw new Error(
Expand Down Expand Up @@ -360,9 +364,10 @@ export class StylingBuilder {
}

private _buildSingleInputs(
reference: o.ExternalReference, inputs: BoundStylingEntry[], valueConverter: ValueConverter,
getInterpolationExpressionFn: ((value: Interpolation) => o.ExternalReference)|null,
isClassBased: boolean): StylingInstruction[] {
reference: o.ExternalReference, inputs: BoundStylingEntry[], mapIndex: Map<string, number>,
allowUnits: boolean, valueConverter: ValueConverter,
getInterpolationExpressionFn?: (value: Interpolation) => o.ExternalReference):
StylingInstruction[] {
const instructions: StylingInstruction[] = [];

inputs.forEach(input => {
Expand All @@ -385,7 +390,7 @@ export class StylingBuilder {
allocateBindingSlots: totalBindingSlotsRequired,
supportsInterpolation: !!getInterpolationExpressionFn,
params: (convertFn: (value: any) => o.Expression | o.Expression[]) => {
// params => stylingProp(propName, value, suffix|sanitizer)
// params => stylingProp(propName, value)
const params: o.Expression[] = [];
params.push(o.literal(input.name));

Expand All @@ -396,16 +401,8 @@ export class StylingBuilder {
params.push(convertResult);
}

// [style.prop] bindings may use suffix values (e.g. px, em, etc...) and they
// can also use a sanitizer. Sanitization occurs for url-based entries. Having
// the suffix value and a sanitizer together into the instruction doesn't make
// any sense (url-based entries cannot be sanitized).
if (!isClassBased) {
if (input.unit) {
params.push(o.literal(input.unit));
} else if (input.sanitize) {
params.push(o.importExpr(R3.defaultStyleSanitizer));
}
if (allowUnits && input.unit) {
params.push(o.literal(input.unit));
}

return params;
Expand All @@ -430,27 +427,41 @@ export class StylingBuilder {
private _buildClassInputs(valueConverter: ValueConverter): StylingInstruction[] {
if (this._singleClassInputs) {
return this._buildSingleInputs(
R3.classProp, this._singleClassInputs, valueConverter, null, true);
R3.classProp, this._singleClassInputs, this._classesIndex, false, valueConverter);
}
return [];
}

private _buildStyleInputs(valueConverter: ValueConverter): StylingInstruction[] {
if (this._singleStyleInputs) {
return this._buildSingleInputs(
R3.styleProp, this._singleStyleInputs, valueConverter,
getStylePropInterpolationExpression, false);
R3.styleProp, this._singleStyleInputs, this._stylesIndex, true, valueConverter,
getStylePropInterpolationExpression);
}
return [];
}

private _buildSanitizerFn(): StylingInstruction {
return {
reference: R3.styleSanitizer,
calls: [{
sourceSpan: this._firstStylingInput ? this._firstStylingInput.sourceSpan : null,
allocateBindingSlots: 0,
params: () => [o.importExpr(R3.defaultStyleSanitizer)]
}]
};
}

/**
* Constructs all instructions which contain the expressions that will be placed
* into the update block of a template function or a directive hostBindings function.
*/
buildUpdateLevelInstructions(valueConverter: ValueConverter) {
const instructions: StylingInstruction[] = [];
if (this.hasBindings) {
if (this._useDefaultSanitizer) {
instructions.push(this._buildSanitizerFn());
}
const styleMapInstruction = this.buildStyleMapInstruction(valueConverter);
if (styleMapInstruction) {
instructions.push(styleMapInstruction);
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/core_render3_private_export.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ export {
ɵɵelementContainerEnd,
ɵɵelementContainer,
ɵɵstyleMap,
ɵɵstyleSanitizer,
ɵɵclassMap,
ɵɵclassMapInterpolate1,
ɵɵclassMapInterpolate2,
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/render3/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ export {
ɵɵstylePropInterpolate8,
ɵɵstylePropInterpolateV,

ɵɵstyleSanitizer,
ɵɵtemplate,

ɵɵtext,
Expand Down
Loading

0 comments on commit 32489c7

Please sign in to comment.