Skip to content

Commit 9d4111d

Browse files
tboschtbosch
committed
fix(compiler): make text interpolation more robust
Allows to add or remove previous siblings of text interpolations (e.g. by added `<script>` tags for content reproduction, or by removed `<style>` tags). Also calculates correctly whether an element is empty. Fixes #2591
1 parent 180e617 commit 9d4111d

6 files changed

Lines changed: 95 additions & 25 deletions

File tree

‎modules/angular2/src/render/dom/compiler/text_interpolation_parser.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export class TextInterpolationParser implements CompileStep {
2626
var expr = this._parser.parseInterpolation(text, current.elementDescription);
2727
if (isPresent(expr)) {
2828
DOM.setText(node, ' ');
29-
current.bindElement().bindText(i, expr);
29+
current.bindElement().bindText(node, expr);
3030
}
3131
}
3232
}

‎modules/angular2/src/render/dom/view/proto_view.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,14 @@ export class DomProtoView {
2626
boundTextNodeCount: number;
2727
rootNodeCount: number;
2828

29-
constructor({elementBinders, element, transitiveContentTagCount}) {
29+
constructor({elementBinders, element, transitiveContentTagCount, boundTextNodeCount}) {
3030
this.element = element;
3131
this.elementBinders = elementBinders;
3232
this.transitiveContentTagCount = transitiveContentTagCount;
3333
this.isTemplateElement = DOM.isTemplateElement(this.element);
3434
this.rootBindingOffset =
3535
(isPresent(this.element) && DOM.hasClass(this.element, NG_BINDING_CLASS)) ? 1 : 0;
36-
this.boundTextNodeCount =
37-
ListWrapper.reduce(elementBinders, (prevCount: number, elementBinder: ElementBinder) =>
38-
prevCount + elementBinder.textNodeIndices.length,
39-
0);
36+
this.boundTextNodeCount = boundTextNodeCount;
4037
this.rootNodeCount =
4138
this.isTemplateElement ? DOM.childNodes(DOM.content(this.element)).length : 1;
4239
}

‎modules/angular2/src/render/dom/view/proto_view_builder.ts‎

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export class ProtoViewBuilder {
4848

4949
var apiElementBinders = [];
5050
var transitiveContentTagCount = 0;
51+
var boundTextNodeCount = 0;
5152
ListWrapper.forEach(this.elements, (ebb: ElementBinderBuilder) => {
5253
var propertySetters = new Map();
5354
var hostActions = new Map();
@@ -103,9 +104,10 @@ export class ProtoViewBuilder {
103104
textBindings: ebb.textBindings,
104105
readAttributes: ebb.readAttributes
105106
}));
106-
var elementIsEmpty = this._isEmptyElement(ebb.element);
107+
var childNodeInfo = this._analyzeChildNodes(ebb.element, ebb.textBindingNodes);
108+
boundTextNodeCount += ebb.textBindingNodes.length;
107109
renderElementBinders.push(new ElementBinder({
108-
textNodeIndices: ebb.textBindingIndices,
110+
textNodeIndices: childNodeInfo.boundTextNodeIndices,
109111
contentTagSelector: ebb.contentTagSelector,
110112
parentIndex: parentIndex,
111113
distanceToParent: ebb.distanceToParent,
@@ -117,34 +119,50 @@ export class ProtoViewBuilder {
117119
globalEvents: ebb.eventBuilder.buildGlobalEvents(),
118120
hostActions: hostActions,
119121
propertySetters: propertySetters,
120-
elementIsEmpty: elementIsEmpty
122+
elementIsEmpty: childNodeInfo.elementIsEmpty
121123
}));
122124
});
123125
return new api.ProtoViewDto({
124126
render: new DomProtoViewRef(new DomProtoView({
125127
element: this.rootElement,
126128
elementBinders: renderElementBinders,
127-
transitiveContentTagCount: transitiveContentTagCount
129+
transitiveContentTagCount: transitiveContentTagCount,
130+
boundTextNodeCount: boundTextNodeCount
128131
})),
129132
type: this.type,
130133
elementBinders: apiElementBinders,
131134
variableBindings: this.variableBindings
132135
});
133136
}
134137

135-
_isEmptyElement(el) {
136-
var childNodes = DOM.childNodes(el);
138+
// Note: We need to calculate the next node indices not until the compilation is complete,
139+
// as the compiler might change the order of elements.
140+
private _analyzeChildNodes(parentElement: /*element*/ any,
141+
boundTextNodes: List</*node*/ any>): _ChildNodesInfo {
142+
var childNodes = DOM.childNodes(DOM.templateAwareRoot(parentElement));
143+
var boundTextNodeIndices = [];
144+
var indexInBoundTextNodes = 0;
145+
var elementIsEmpty = true;
137146
for (var i = 0; i < childNodes.length; i++) {
138147
var node = childNodes[i];
139-
if ((DOM.isTextNode(node) && DOM.getText(node).trim().length > 0) ||
140-
(DOM.isElementNode(node))) {
141-
return false;
148+
if (indexInBoundTextNodes < boundTextNodes.length &&
149+
node === boundTextNodes[indexInBoundTextNodes]) {
150+
boundTextNodeIndices.push(i);
151+
indexInBoundTextNodes++;
152+
elementIsEmpty = false;
153+
} else if ((DOM.isTextNode(node) && DOM.getText(node).trim().length > 0) ||
154+
(DOM.isElementNode(node))) {
155+
elementIsEmpty = false;
142156
}
143157
}
144-
return true;
158+
return new _ChildNodesInfo(boundTextNodeIndices, elementIsEmpty);
145159
}
146160
}
147161

162+
class _ChildNodesInfo {
163+
constructor(public boundTextNodeIndices: List<number>, public elementIsEmpty: boolean) {}
164+
}
165+
148166
export class ElementBinderBuilder {
149167
parent: ElementBinderBuilder = null;
150168
distanceToParent: number = 0;
@@ -154,7 +172,7 @@ export class ElementBinderBuilder {
154172
variableBindings: Map<string, string> = new Map();
155173
eventBindings: List<api.EventBinding> = [];
156174
eventBuilder: EventBuilder = new EventBuilder();
157-
textBindingIndices: List<number> = [];
175+
textBindingNodes: List</*node*/ any> = [];
158176
textBindings: List<ASTWithSource> = [];
159177
contentTagSelector: string = null;
160178
readAttributes: Map<string, string> = new Map();
@@ -214,8 +232,8 @@ export class ElementBinderBuilder {
214232
this.eventBindings.push(this.eventBuilder.add(name, expression, target));
215233
}
216234

217-
bindText(index, expression) {
218-
this.textBindingIndices.push(index);
235+
bindText(textNode, expression) {
236+
this.textBindingNodes.push(textNode);
219237
this.textBindings.push(expression);
220238
}
221239

‎modules/angular2/test/render/dom/compiler/text_interpolation_parser_spec.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {MapWrapper, ListWrapper} from 'angular2/src/facade/collection';
55
import {Lexer, Parser} from 'angular2/change_detection';
66
import {IgnoreChildrenStep} from './pipeline_spec';
77
import {ElementBinderBuilder} from 'angular2/src/render/dom/view/proto_view_builder';
8+
import {DOM} from 'angular2/src/dom/dom_adapter';
89

910
export function main() {
1011
describe('TextInterpolationParser', () => {
@@ -20,7 +21,8 @@ export function main() {
2021

2122
function assertTextBinding(elementBinder, bindingIndex, nodeIndex, expression) {
2223
expect(elementBinder.textBindings[bindingIndex].source).toEqual(expression);
23-
expect(elementBinder.textBindingIndices[bindingIndex]).toEqual(nodeIndex);
24+
expect(elementBinder.textBindingNodes[bindingIndex])
25+
.toEqual(DOM.childNodes(DOM.templateAwareRoot(elementBinder.element))[nodeIndex]);
2426
}
2527

2628
it('should find text interpolation in normal elements', () => {

‎modules/angular2/test/render/dom/shadow_dom_emulation_integration_spec.ts‎

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,16 @@ import {DomTestbed} from './dom_testbed';
3636

3737
export function main() {
3838
describe('ShadowDom integration tests', function() {
39+
var styleHost;
3940
var strategies = {
4041
"scoped":
4142
bind(ShadowDomStrategy)
4243
.toFactory((styleInliner, styleUrlResolver) => new EmulatedScopedShadowDomStrategy(
43-
styleInliner, styleUrlResolver, null),
44+
styleInliner, styleUrlResolver, styleHost),
4445
[StyleInliner, StyleUrlResolver]),
4546
"unscoped": bind(ShadowDomStrategy)
46-
.toFactory((styleUrlResolver) =>
47-
new EmulatedUnscopedShadowDomStrategy(styleUrlResolver, null),
47+
.toFactory((styleUrlResolver) => new EmulatedUnscopedShadowDomStrategy(
48+
styleUrlResolver, styleHost),
4849
[StyleUrlResolver])
4950
};
5051
if (DOM.supportsNativeShadowDOM()) {
@@ -54,12 +55,15 @@ export function main() {
5455
.toFactory((styleUrlResolver) => new NativeShadowDomStrategy(styleUrlResolver),
5556
[StyleUrlResolver]));
5657
}
58+
beforeEach(() => { styleHost = el('<div></div>'); });
5759

5860
StringMapWrapper.forEach(strategies, (strategyBinding, name) => {
5961
describe(`${name} shadow dom strategy`, () => {
6062
beforeEachBindings(() => { return [strategyBinding, DomTestbed]; });
6163

6264
// GH-2095 - https://github.com/angular/angular/issues/2095
65+
// important as we are adding a content end element during compilation,
66+
// which could skrew up text node indices.
6367
it('should support text nodes after content tags',
6468
inject([DomTestbed, AsyncTestCompleter], (tb, async) => {
6569
tb.compileAll([
@@ -80,6 +84,28 @@ export function main() {
8084
});
8185
}));
8286

87+
// important as we are moving style tags around during compilation,
88+
// which could skrew up text node indices.
89+
it('should support text nodes after style tags',
90+
inject([DomTestbed, AsyncTestCompleter], (tb, async) => {
91+
tb.compileAll([
92+
simple,
93+
new ViewDefinition({
94+
componentId: 'simple',
95+
template: '<style></style><p>P,</p>{{a}}',
96+
directives: []
97+
})
98+
])
99+
.then((protoViewDtos) => {
100+
var rootView = tb.createRootView(protoViewDtos[0]);
101+
var cmpView = tb.createComponentView(rootView.viewRef, 0, protoViewDtos[1]);
102+
103+
tb.renderer.setText(cmpView.viewRef, 0, 'text');
104+
expect(tb.rootEl).toHaveText('P,text');
105+
async.done();
106+
});
107+
}));
108+
83109
it('should support simple components',
84110
inject([AsyncTestCompleter, DomTestbed], (async, tb) => {
85111
tb.compileAll([
@@ -102,6 +128,29 @@ export function main() {
102128
});
103129
}));
104130

131+
it('should support simple components with text interpolation as direct children',
132+
inject([AsyncTestCompleter, DomTestbed], (async, tb) => {
133+
tb.compileAll([
134+
mainDir,
135+
new ViewDefinition({
136+
componentId: 'main',
137+
template: '<simple>' +
138+
'{{text}}' +
139+
'</simple>',
140+
directives: [simple]
141+
}),
142+
simpleTemplate
143+
])
144+
.then((protoViews) => {
145+
var cmpView = tb.createRootViews(protoViews)[1];
146+
tb.renderer.setText(cmpView.viewRef, 0, 'A');
147+
148+
expect(tb.rootEl).toHaveText('SIMPLE(A)');
149+
150+
async.done();
151+
});
152+
}));
153+
105154
it('should not show the light dom even if there is not content tag',
106155
inject([AsyncTestCompleter, DomTestbed], (async, tb) => {
107156
tb.compileAll([

‎modules/angular2/test/render/dom/view/view_spec.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,12 @@ export function main() {
3030
binders = [];
3131
}
3232
var rootEl = el('<div></div>');
33-
return new DomProtoView(
34-
{element: rootEl, elementBinders: binders, transitiveContentTagCount: 0});
33+
return new DomProtoView({
34+
element: rootEl,
35+
elementBinders: binders,
36+
transitiveContentTagCount: 0,
37+
boundTextNodeCount: 0
38+
});
3539
}
3640

3741
function createView(pv = null, boundElementCount = 0) {

0 commit comments

Comments
 (0)