From 79659ee5aa540637d08ca2d40e3258487c03c1dc Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Mon, 9 Mar 2020 21:49:02 -0700 Subject: [PATCH] fix(compiler): support directive inputs with interpolations on ``s (#35984) Prior to this commit, Ivy compiler didn't handle directive inputs with interpolations located on `` elements (e.g. ``). That was the case for regular inputs as well as inputs that should be processed via i18n subsystem (e.g. ``). This commit adds support for such expressions for explicit ``s as well as a number of tests to confirm the behavior. Fixes #35752. PR Close #35984 --- .../compliance/r3_view_compiler_i18n_spec.ts | 80 +++++++++++++++++ .../r3_view_compiler_template_spec.ts | 89 +++++++++++++++++++ .../compiler/src/render3/view/template.ts | 42 ++++++--- .../core/test/acceptance/directive_spec.ts | 72 +++++++++++++++ packages/core/test/acceptance/i18n_spec.ts | 52 +++++++++++ 5 files changed, 323 insertions(+), 12 deletions(-) diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts index 9b449b49f9..935859f2df 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts @@ -403,6 +403,86 @@ describe('i18n support in the template compiler', () => { verify(input, output); }); + it('should support i18n attributes with interpolations on explicit elements', + () => { + const input = ` + + `; + + const output = String.raw ` + var $I18N_0$; + if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) { + const $MSG_EXTERNAL_3771704108176831903$$APP_SPEC_TS_1$ = goog.getMsg("Hello {$interpolation}", { + "interpolation": "\uFFFD0\uFFFD" + }); + $I18N_0$ = $MSG_EXTERNAL_3771704108176831903$$APP_SPEC_TS_1$; + } + else { + $I18N_0$ = $localize \`Hello $` + + String.raw `{"\uFFFD0\uFFFD"}:INTERPOLATION:\`; + } + const $_c2$ = ["title", $I18N_0$]; + … + consts: [[${AttributeMarker.Bindings}, "title"]], + template: function MyComponent_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵtemplate(0, MyComponent_ng_template_0_Template, 0, 0, "ng-template", 0); + $r3$.ɵɵi18nAttributes(1, $_c2$); + } + if (rf & 2) { + $r3$.ɵɵi18nExp(ctx.name); + $r3$.ɵɵi18nApply(1); + } + } + `; + verify(input, output); + }); + + it('should support i18n attributes with interpolations on explicit elements with structural directives', + () => { + const input = ` + + `; + + const output = String.raw ` + var $I18N_0$; + if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) { + const $MSG_EXTERNAL_3771704108176831903$$APP_SPEC_TS__1$ = goog.getMsg("Hello {$interpolation}", { + "interpolation": "\uFFFD0\uFFFD" + }); + $I18N_0$ = $MSG_EXTERNAL_3771704108176831903$$APP_SPEC_TS__1$; + } + else { + $I18N_0$ = $localize \`Hello $` + + String.raw `{"\uFFFD0\uFFFD"}:INTERPOLATION:\`; + } + const $_c2$ = ["title", $I18N_0$]; + … + function MyComponent_0_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵtemplate(0, MyComponent_0_ng_template_0_Template, 0, 0, "ng-template", 1); + $r3$.ɵɵi18nAttributes(1, $_c2$); + } + if (rf & 2) { + const $ctx_r2$ = $r3$.ɵɵnextContext(); + $r3$.ɵɵi18nExp($ctx_r2$.name); + $r3$.ɵɵi18nApply(1); + } + } + … + consts: [[${AttributeMarker.Template}, "ngIf"], [${AttributeMarker.Bindings}, "title"]], + template: function MyComponent_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵtemplate(0, MyComponent_0_Template, 2, 1, undefined, 0); + } + if (rf & 2) { + $r3$.ɵɵproperty("ngIf", true); + } + }, + `; + verify(input, output); + }); + it('should not create translations for empty attributes', () => { const input = `
diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_template_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_template_spec.ts index 2ae7af80e0..c6e795d9ba 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_template_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_template_spec.ts @@ -548,6 +548,95 @@ describe('compiler compliance: template', () => { }); + it('should allow directive inputs as an interpolated prop on ', () => { + const files = { + app: { + 'spec.ts': ` + import {Component, Directive, Input} from '@angular/core'; + + @Directive({selector: '[dir]'}) + class WithInput { + @Input() dir: string = ''; + } + + @Component({ + selector: 'my-app', + template: '', + }) + export class TestComp { + message = 'Hello'; + } + ` + } + }; + const result = compile(files, angularFiles); + const expectedTemplate = ` + consts: [[${AttributeMarker.Bindings}, "dir"]], + template: function TestComp_Template(rf, ctx) { + if (rf & 1) { + $i0$.ɵɵtemplate(0, $TestComp_ng_template_0_Template$, 0, 0, "ng-template", 0); + } + if (rf & 2) { + $i0$.ɵɵpropertyInterpolate("dir", ctx.message); + } + }, + `; + expectEmit(result.source, expectedTemplate, 'Incorrect template'); + }); + + it('should allow directive inputs as an interpolated prop on (with structural directives)', + () => { + const files = { + app: { + 'spec.ts': ` + import {Component, Directive, Input} from '@angular/core'; + + @Directive({selector: '[dir]'}) + class WithInput { + @Input() dir: string = ''; + } + + @Component({ + selector: 'my-app', + template: '', + }) + export class TestComp { + message = 'Hello'; + } + ` + } + }; + const result = compile(files, angularFiles); + + // Expect that `ɵɵpropertyInterpolate` is generated in the inner template function. + const expectedInnerTemplate = ` + function $TestComp_0_Template$(rf, ctx) { + if (rf & 1) { + $i0$.ɵɵtemplate(0, $TestComp_0_ng_template_0_Template$, 0, 0, "ng-template", 1); + } + if (rf & 2) { + const $ctx_r0$ = i0.ɵɵnextContext(); + $i0$.ɵɵpropertyInterpolate("dir", $ctx_r0$.message); + } + } + `; + expectEmit(result.source, expectedInnerTemplate, 'Incorrect template'); + + // Main template should just contain *ngIf property. + const expectedMainTemplate = ` + consts: [[4, "ngIf"], [3, "dir"]], + template: function TestComp_Template(rf, ctx) { + if (rf & 1) { + $i0$.ɵɵtemplate(0, $TestComp_0_Template$, 1, 1, undefined, 0); + } + if (rf & 2) { + $i0$.ɵɵproperty("ngIf", true); + } + }, + `; + expectEmit(result.source, expectedMainTemplate, 'Incorrect template'); + }); + it('should create unique template function names even for similar nested template structures', () => { const files = { diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index abea5c6fbe..18ad2a784d 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -587,11 +587,7 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver } else if (name === 'class') { stylingBuilder.registerClassAttr(value); } else { - if (attr.i18n) { - i18nAttrs.push(attr); - } else { - outputAttrs.push(attr); - } + (attr.i18n ? i18nAttrs : outputAttrs).push(attr); } } @@ -895,17 +891,26 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver // Only add normal input/output binding instructions on explicit elements. if (template.tagName === NG_TEMPLATE_TAG_NAME) { + const inputs: t.BoundAttribute[] = []; + const i18nAttrs: (t.TextAttribute | t.BoundAttribute)[] = + template.attributes.filter(attr => !!attr.i18n); + + template.inputs.forEach( + (input: t.BoundAttribute) => (input.i18n ? i18nAttrs : inputs).push(input)); + // Add i18n attributes that may act as inputs to directives. If such attributes are present, // generate `i18nAttributes` instruction. Note: we generate it only for explicit // elements, in case of inline templates, corresponding instructions will be generated in the // nested template function. - const i18nAttrs: t.TextAttribute[] = template.attributes.filter(attr => !!attr.i18n); if (i18nAttrs.length > 0) { this.i18nAttributesInstruction(templateIndex, i18nAttrs, template.sourceSpan); } // Add the input bindings - this.templatePropertyBindings(templateIndex, template.inputs); + if (inputs.length > 0) { + this.templatePropertyBindings(templateIndex, inputs); + } + // Generate listeners for directive output if (template.outputs.length > 0) { const listeners = template.outputs.map( @@ -1036,11 +1041,24 @@ export class TemplateDefinitionBuilder implements t.Visitor, LocalResolver if (value !== undefined) { this.allocateBindingSlots(value); - propertyBindings.push({ - name: input.name, - sourceSpan: input.sourceSpan, - value: () => this.convertPropertyBinding(value) - }); + if (value instanceof Interpolation) { + // Params typically contain attribute namespace and value sanitizer, which is applicable + // for regular HTML elements, but not applicable for (since props act as + // inputs to directives), so keep params array empty. + const params: any[] = []; + + // prop="{{value}}" case + this.interpolatedUpdateInstruction( + getPropertyInterpolationExpression(value), templateIndex, input.name, input, value, + params); + } else { + // [prop]="value" case + propertyBindings.push({ + name: input.name, + sourceSpan: input.sourceSpan, + value: () => this.convertPropertyBinding(value) + }); + } } } }); diff --git a/packages/core/test/acceptance/directive_spec.ts b/packages/core/test/acceptance/directive_spec.ts index 2903f56d1a..437cf825f2 100644 --- a/packages/core/test/acceptance/directive_spec.ts +++ b/packages/core/test/acceptance/directive_spec.ts @@ -341,6 +341,78 @@ describe('directives', () => { }); + describe('inputs', () => { + it('should allow directive inputs (as a prop binding) on ', () => { + let dirInstance: WithInput; + @Directive({selector: '[dir]'}) + class WithInput { + constructor() { dirInstance = this; } + @Input() dir: string = ''; + } + + @Component({ + selector: 'my-app', + template: '', + }) + class TestComp { + message = 'Hello'; + } + + TestBed.configureTestingModule({declarations: [TestComp, WithInput]}); + const fixture = TestBed.createComponent(TestComp); + fixture.detectChanges(); + + expect(dirInstance !.dir).toBe('Hello'); + }); + + it('should allow directive inputs (as an interpolated prop) on ', () => { + let dirInstance: WithInput; + @Directive({selector: '[dir]'}) + class WithInput { + constructor() { dirInstance = this; } + @Input() dir: string = ''; + } + + @Component({ + selector: 'my-app', + template: '', + }) + class TestComp { + message = 'Hello'; + } + + TestBed.configureTestingModule({declarations: [TestComp, WithInput]}); + const fixture = TestBed.createComponent(TestComp); + fixture.detectChanges(); + + expect(dirInstance !.dir).toBe('Hello'); + }); + + it('should allow directive inputs (as an interpolated prop) on with structural directives', + () => { + let dirInstance: WithInput; + @Directive({selector: '[dir]'}) + class WithInput { + constructor() { dirInstance = this; } + @Input() dir: string = ''; + } + + @Component({ + selector: 'my-app', + template: '', + }) + class TestComp { + message = 'Hello'; + } + + TestBed.configureTestingModule({declarations: [TestComp, WithInput]}); + const fixture = TestBed.createComponent(TestComp); + fixture.detectChanges(); + + expect(dirInstance !.dir).toBe('Hello'); + }); + }); + describe('outputs', () => { @Directive({selector: '[out]'}) class TestDir { diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts index c32c56f02b..bf598a8868 100644 --- a/packages/core/test/acceptance/i18n_spec.ts +++ b/packages/core/test/acceptance/i18n_spec.ts @@ -1423,6 +1423,58 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { expect(titleDirInstances[0].title).toBe('Bonjour'); }); + it('should allow directive inputs (as an interpolated prop) on ', () => { + loadTranslations({[computeMsgId('Hello {$INTERPOLATION}')]: 'Bonjour {$INTERPOLATION}'}); + + let dirInstance: WithInput; + @Directive({selector: '[dir]'}) + class WithInput { + constructor() { dirInstance = this; } + @Input() dir: string = ''; + } + + @Component({ + selector: 'my-app', + template: '', + }) + class TestComp { + name = 'Angular'; + } + + TestBed.configureTestingModule({declarations: [TestComp, WithInput]}); + const fixture = TestBed.createComponent(TestComp); + fixture.detectChanges(); + + expect(dirInstance !.dir).toBe('Bonjour Angular'); + }); + + it('should allow directive inputs (as interpolated props)' + + 'on with structural directives present', + () => { + loadTranslations({[computeMsgId('Hello {$INTERPOLATION}')]: 'Bonjour {$INTERPOLATION}'}); + + let dirInstance: WithInput; + @Directive({selector: '[dir]'}) + class WithInput { + constructor() { dirInstance = this; } + @Input() dir: string = ''; + } + + @Component({ + selector: 'my-app', + template: '', + }) + class TestComp { + name = 'Angular'; + } + + TestBed.configureTestingModule({declarations: [TestComp, WithInput]}); + const fixture = TestBed.createComponent(TestComp); + fixture.detectChanges(); + + expect(dirInstance !.dir).toBe('Bonjour Angular'); + }); + it('should apply i18n attributes during second template pass', () => { loadTranslations({[computeMsgId('Set')]: 'Set'}); @Directive({