From 07069c69057731730d25bfe340f843db8b3c23cc Mon Sep 17 00:00:00 2001 From: Godhuda Date: Thu, 21 Jan 2016 18:59:44 -0800 Subject: [PATCH] Initial implementation of element merge strategy This commit supports attribute merging in component invocations by storing the attribute operations for an element in GroupedElementOperations. In `CloseElementOpcode`, we merge all of the seen operations and execute them. In principle, this work is only needed for component elements, which can have multiple sources of attributes with arbitrary merge strategies. This commit adds the GroupedElementOperations to every element, which mildly regresses performance. We plan to limit the impact to component elements, and have plans to avoid the work in all but the most dynamic scenarios. That said, this works and passes a new somewhat aggressive smoke test based on one that helped us understand the nature of the problem in Glimmer 1. Near-term work: * Avoid GroupedElementOperations for regular elements * Avoid emitting UpdatingOpcodes for static attributes on components * Extract merge strategy into ComponentManager (and possibly allow kinds of attributes to control their own merging directly) * Correctly group the operations so that invoker and invokee attributes can be properly interpreted by the merging logic. --- .../lib/javascript-compiler.ts | 5 - .../glimmer-compiler/lib/template-compiler.ts | 5 +- .../node_modules/glimmer-runtime/index.ts | 1 - .../glimmer-runtime/lib/builder.ts | 132 +++++---- .../lib/compiled/opcodes/component.ts | 15 +- .../lib/compiled/opcodes/dom.ts | 262 ++++++++++++------ .../node_modules/glimmer-runtime/lib/dom.ts | 8 +- .../glimmer-runtime/lib/syntax/core.ts | 41 --- .../glimmer-runtime/lib/syntax/statements.ts | 3 - .../tests/ember-component-test.ts | 72 ++++- .../glimmer-test-helpers/lib/environment.ts | 7 +- .../node_modules/glimmer-wire-format/index.ts | 3 - 12 files changed, 335 insertions(+), 219 deletions(-) diff --git a/packages/node_modules/glimmer-compiler/lib/javascript-compiler.ts b/packages/node_modules/glimmer-compiler/lib/javascript-compiler.ts index e7869fee12..c126524105 100644 --- a/packages/node_modules/glimmer-compiler/lib/javascript-compiler.ts +++ b/packages/node_modules/glimmer-compiler/lib/javascript-compiler.ts @@ -132,11 +132,6 @@ export default class JavaScriptCompiler { this.push(['closeElement']); } - addClass() { - let value = this.popValue(); - this.push(['addClass', value]); - } - staticAttr(name: str, namespace: str) { let value = this.popValue(); this.push(['staticAttr', name, value, namespace]); diff --git a/packages/node_modules/glimmer-compiler/lib/template-compiler.ts b/packages/node_modules/glimmer-compiler/lib/template-compiler.ts index 27e3f3ba3f..74a3b6e36d 100644 --- a/packages/node_modules/glimmer-compiler/lib/template-compiler.ts +++ b/packages/node_modules/glimmer-compiler/lib/template-compiler.ts @@ -80,10 +80,7 @@ export default class TemplateCompiler { let isStatic = this.prepareAttributeValue(value); - // REFACTOR TODO: escaped? - if (name === 'class') { - this.opcode('addClass', action); - } else if (isStatic) { + if (isStatic) { this.opcode('staticAttr', action, name, namespace); } else if (action.value.type === 'MustacheStatement') { this.opcode('dynamicProp', action, name); diff --git a/packages/node_modules/glimmer-runtime/index.ts b/packages/node_modules/glimmer-runtime/index.ts index 96519296df..160da7055c 100644 --- a/packages/node_modules/glimmer-runtime/index.ts +++ b/packages/node_modules/glimmer-runtime/index.ts @@ -19,7 +19,6 @@ export { StaticAttr, DynamicAttr, DynamicProp, - AddClass, Args as ArgsSyntax, NamedArgs as NamedArgsSyntax, PositionalArgs as PositionalArgsSyntax, diff --git a/packages/node_modules/glimmer-runtime/lib/builder.ts b/packages/node_modules/glimmer-runtime/lib/builder.ts index 5515a8dc61..1c6c56914e 100644 --- a/packages/node_modules/glimmer-runtime/lib/builder.ts +++ b/packages/node_modules/glimmer-runtime/lib/builder.ts @@ -5,10 +5,16 @@ import DOMHelper from './dom'; import { InternedString, Stack, LinkedList, LinkedListNode, assert } from 'glimmer-util'; import { - ChainableReference, - PushPullReference, + PathReference } from 'glimmer-reference'; +import { + ElementOperation, + NamespacedAttribute, + NonNamespacedAttribute, + Property +} from './compiled/opcodes/dom'; + interface FirstNode { firstNode(): Node; } @@ -41,24 +47,6 @@ class Last { } } -export class ClassList extends PushPullReference { - private list: ChainableReference[] = []; - - isEmpty() { - return this.list.length === 0; - } - - append(reference: ChainableReference) { - this.list.push(reference); - // this._addSource(reference); - } - - value(): string { - if (this.list.length === 0) return null; - return this.list.map(i => i.value()).join(' '); - } -} - interface ElementStackOptions { parentNode: Element; nextSibling: Node; @@ -74,15 +62,42 @@ class BlockStackElement { public lastNode: Node = null; } +class GroupedElementOperations { + groups: ElementOperation[][]; + group: ElementOperation[]; + + constructor() { + let group = this.group = []; + this.groups = [group]; + } + + startGroup() { + let group = this.group = []; + this.groups.push(group); + } + + addAttribute(name: InternedString, value: PathReference) { + this.group.push(new NonNamespacedAttribute(name, value)); + } + + addAttributeNS(name: InternedString, value: PathReference, namespace: InternedString) { + this.group.push(new NamespacedAttribute(name, value, namespace)); + } + + addProperty(name: InternedString, value: PathReference) { + this.group.push(new Property(name, value)); + } +} + export class ElementStack { public nextSibling: Node; public dom: DOMHelper; public element: Element; - public classList: ClassList = null; + public elementOperations: GroupedElementOperations = null; private elementStack = new Stack(); private nextSiblingStack = new Stack(); - private classListStack = new Stack(); + private elementOperationsStack = new Stack(); private blockStack = new Stack(); constructor({ dom, parentNode, nextSibling }: ElementStackOptions) { @@ -99,26 +114,31 @@ export class ElementStack { return this.blockStack.current; } - private pushElement(element) { - this.classList = new ClassList(); + private pushElement(tag: InternedString): Element { + let element = this.dom.createElement(tag, this.element); + let elementOperations = new GroupedElementOperations(); + + this.elementOperations = elementOperations; this.element = element; this.nextSibling = null; this.elementStack.push(element); - this.classListStack.push(this.classList); + this.elementOperationsStack.push(elementOperations) this.nextSiblingStack.push(null); + + return element; } private popElement() { - let { elementStack, nextSiblingStack, classListStack } = this; - let topElement = elementStack.pop(); + let { elementStack, nextSiblingStack, elementOperationsStack } = this; + let topElement = elementStack.pop(); nextSiblingStack.pop(); - classListStack.pop(); + elementOperationsStack.pop(); this.element = elementStack.current; this.nextSibling = nextSiblingStack.current; - this.classList = classListStack.current; + this.elementOperations = elementOperationsStack.current; return topElement; } @@ -145,9 +165,8 @@ export class ElementStack { return this.blockStack.pop(); } - openElement(tag: string): Element { - let element = this.dom.createElement(tag, this.element); - this.pushElement(element); + openElement(tag: InternedString): Element { + let element = this.pushElement(tag); this.blockStack.current.openElement(element); return element; } @@ -185,40 +204,41 @@ export class ElementStack { } insertHTMLBefore(nextSibling: Node, html: string): Bounds { - if (!(this.element instanceof HTMLElement)) { - throw new Error(`You cannot insert HTML (using triple-curlies or htmlSafe) into an SVG context: ${this.element.tagName}`); + let element = this.element; + + if (!canInsertHTML(element)) { + throw new Error(`You cannot insert HTML (using triple-curlies or htmlSafe) into an SVG context: ${element.tagName}`); + } else { + let bounds = this.dom.insertHTMLBefore(element, nextSibling, html); + this.blockStack.current.newBounds(bounds); + return bounds; } - - let bounds = this.dom.insertHTMLBefore(this.element, nextSibling, html); - this.blockStack.current.newBounds(bounds); - return bounds; } - setAttribute(name: InternedString, value: any) { - this.dom.setAttribute(this.element, name, value); + // setAttribute(name: InternedString, value: any) { + // this.dom.setAttribute(this.element, name, value); + // } + + // setAttributeNS(name: InternedString, value: any, namespace: InternedString) { + // this.dom.setAttributeNS(this.element, name, value, namespace); + // } + + setAttribute(name: InternedString, value: PathReference) { + this.elementOperations.addAttribute(name, value); } - setAttributeNS(name: InternedString, value: any, namespace: InternedString) { - this.dom.setAttributeNS(this.element, name, value, namespace); + setAttributeNS(name: InternedString, value: PathReference, namespace: InternedString) { + this.elementOperations.addAttributeNS(name, value, namespace); } - addClass(ref: ChainableReference) { - this.classList.append(ref); + setProperty(name: InternedString, value: PathReference) { + this.elementOperations.addProperty(name, value); } - closeElement(): { element: Element, classList: ClassList, classNames: string } { - let { classList } = this; + closeElement() { this.blockStack.current.closeElement(); let child = this.popElement(); this.dom.insertBefore(this.element, child, this.nextSibling); - - let classNames = classList.value(); - - if (classNames !== null) { - this.dom.setAttribute(child, 'class', classNames); - } - - return { element: child, classList, classNames }; } appendHTML(html: string): Bounds { @@ -226,6 +246,10 @@ export class ElementStack { } } +function canInsertHTML(node: Node): node is HTMLElement { + return node instanceof HTMLElement; +} + interface Tracker extends Bounds { openElement(element: Element); closeElement(); diff --git a/packages/node_modules/glimmer-runtime/lib/compiled/opcodes/component.ts b/packages/node_modules/glimmer-runtime/lib/compiled/opcodes/component.ts index f24fb06ac0..705dc8b843 100644 --- a/packages/node_modules/glimmer-runtime/lib/compiled/opcodes/component.ts +++ b/packages/node_modules/glimmer-runtime/lib/compiled/opcodes/component.ts @@ -1,6 +1,6 @@ import { Opcode, OpcodeJSON, UpdatingOpcode } from '../../opcodes'; import { Component, ComponentManager, ComponentDefinition } from '../../component/interfaces'; -import { UpdateAttributeOpcode } from './dom'; +import { PatchElementOpcode } from './dom'; import { VM, UpdatingVM } from '../../vm'; import { CompiledArgs, EvaluatedArgs } from '../../compiled/expressions/args'; import { Templates } from '../../syntax/core'; @@ -108,18 +108,7 @@ export class ShadowAttributesOpcode extends Opcode { if (!shadow) return; shadow.forEach(name => { - let reference = named.get(name); - let value = reference.value(); - - if (name === "class") { - vm.stack().addClass(reference); - } else { - vm.stack().setAttribute(name, value); - } - - if (!reference[CONST_REFERENCE]) { - vm.updateWith(new UpdateAttributeOpcode(vm.stack().element, name, reference, value)); - } + vm.stack().setAttribute(name, named.get(name)); }); } diff --git a/packages/node_modules/glimmer-runtime/lib/compiled/opcodes/dom.ts b/packages/node_modules/glimmer-runtime/lib/compiled/opcodes/dom.ts index 52060d904e..5c13a09a8a 100644 --- a/packages/node_modules/glimmer-runtime/lib/compiled/opcodes/dom.ts +++ b/packages/node_modules/glimmer-runtime/lib/compiled/opcodes/dom.ts @@ -1,7 +1,9 @@ import { Opcode, OpcodeJSON, UpdatingOpcode } from '../../opcodes'; import { VM, UpdatingVM } from '../../vm'; -import { InternedString, dict } from 'glimmer-util'; -import { ChainableReference } from 'glimmer-reference'; +import { FIXME, InternedString, dict } from 'glimmer-util'; +import { PathReference, PushPullReference } from 'glimmer-reference'; +import DOMHelper from '../../dom'; +import { ValueReference } from '../../compiled/expressions/value'; abstract class DOMUpdatingOpcode extends UpdatingOpcode { public type: string; @@ -55,35 +57,79 @@ export class OpenPrimitiveElementOpcode extends Opcode { } } +class ClassList extends PushPullReference { + private list: PathReference[] = []; + + isEmpty() { + return this.list.length === 0; + } + + append(reference: PathReference) { + this.list.push(reference); + } + + value(): string { + if (this.list.length === 0) return null; + return this.list.map(i => i.value()).join(' '); + } + + get(): FIXME { + return null; + } +} export class CloseElementOpcode extends Opcode { public type = "close-element"; evaluate(vm: VM) { - let { element, classList, classNames } = vm.stack().closeElement(); - - if (!classList.isEmpty()) { - vm.updateWith(new UpdateAttributeOpcode(element, "class", classList, classNames)); + let dom = vm.env.getDOM(); + let stack = vm.stack(); + let { element, elementOperations: { groups } } = stack; + + let classes = new ClassList(); + let flattened = dict(); + + groups.forEach(group => { + group.forEach(op => { + let name: string = op['name']; + let value: PathReference = op['value']; + + if (name === 'class') { + classes.append(value); + } else { + flattened[name] = op; + } + }); + }); + + if (!classes.isEmpty()) { + vm.updateWith(new NonNamespacedAttribute('class' as InternedString, classes).flush(dom, element)); } + + Object.keys(flattened).forEach(key => { + vm.updateWith(flattened[key].flush(dom, element)); + }); + + stack.closeElement(); } } export class StaticAttrOpcode extends Opcode { public type = "static-attr"; public name: InternedString; - public value: InternedString; + public value: ValueReference; public namespace: InternedString; constructor({ name, value, namespace }: { name: InternedString, value: InternedString, namespace: InternedString }) { super(); this.name = name; - this.value = value; + this.value = new ValueReference(value); this.namespace = namespace; } evaluate(vm: VM) { let { name, value, namespace } = this; - if (this.namespace) { + if (namespace) { vm.stack().setAttributeNS(name, value, namespace); } else { vm.stack().setAttribute(name, value); @@ -106,7 +152,111 @@ export class StaticAttrOpcode extends Opcode { } } -export class DynamicAttrOpcode extends Opcode { +export interface ElementOperation { + flush(dom: DOMHelper, element: Element): UpdatingOpcode; +} + +interface ElementPatchOperation extends ElementOperation { + apply(dom: DOMHelper, element: Element, lastValue: any): any; + toJSON(): string[]; +} + +export class NamespacedAttribute implements ElementPatchOperation { + name: InternedString; + value: PathReference; + namespace: InternedString; + + constructor(name: InternedString, value: PathReference, namespace: InternedString) { + this.name = name; + this.value = value; + this.namespace = namespace; + } + + flush(dom: DOMHelper, element: Element): PatchElementOpcode { + let value = this.apply(dom, element); + return new PatchElementOpcode(element, this, value); + } + + apply(dom: DOMHelper, element: Element, lastValue: string = null): any { + let { value: reference, namespace } = this; + let value = reference.value(); + + if (value === lastValue) { + return lastValue; + } else { + dom.setAttributeNS(element, name, value, namespace); + return value; + } + } + + toJSON(): string[] { + return ['AttributeNS', this.name]; + } +} + +export class NonNamespacedAttribute implements ElementPatchOperation { + name: InternedString; + value: PathReference; + + constructor(name: InternedString, value: PathReference) { + this.name = name; + this.value = value; + } + + flush(dom: DOMHelper, element: Element): PatchElementOpcode { + let value = this.apply(dom, element); + return new PatchElementOpcode(element, this, value); + } + + apply(dom: DOMHelper, element: Element, lastValue: any = null): any { + let { name, value: reference } = this; + let value = reference.value(); + + if (value === lastValue) { + return lastValue; + } else { + dom.setAttribute(element, name, value); + return value; + } + } + + toJSON(): string[] { + return ['Attribute', this.name]; + } +} + +export class Property implements ElementPatchOperation { + name: InternedString; + value: PathReference; + + constructor(name: InternedString, value: PathReference) { + this.name = name; + this.value = value; + } + + flush(dom: DOMHelper, element: Element): PatchElementOpcode { + let value = this.apply(dom, element); + return new PatchElementOpcode(element, this, value); + } + + apply(dom: DOMHelper, element: Element, lastValue: any = null): any { + let { name, value: reference } = this; + let value = reference.value(); + + if (value === lastValue) { + return lastValue; + } else { + dom.setProperty(element, name, value); + return value; + } + } + + toJSON(): string[] { + return ['Property', this.name]; + } +} + +export class DynamicAttrNSOpcode extends Opcode { public type = "dynamic-attr"; public name: InternedString; public namespace: InternedString; @@ -120,15 +270,7 @@ export class DynamicAttrOpcode extends Opcode { evaluate(vm: VM) { let { name, namespace } = this; let reference = vm.frame.getOperand(); - let value = reference.value(); - - if (this.namespace) { - vm.stack().setAttributeNS(name, value, namespace); - } else { - vm.stack().setAttribute(name, value); - } - - vm.updateWith(new UpdateAttributeOpcode(vm.stack().element, name, reference, value)); + vm.stack().setAttributeNS(name, reference, namespace); } toJSON(): OpcodeJSON { @@ -147,49 +289,28 @@ export class DynamicAttrOpcode extends Opcode { } } -export class UpdateAttributeOpcode extends DOMUpdatingOpcode { - public type = "update-attribute"; - - private element: Element; - private name: string; - private namespace: string; - private reference: ChainableReference; - private lastValue: string; +export class DynamicAttrOpcode extends Opcode { + public type = "dynamic-attr"; + public name: InternedString; - constructor(element: Element, name: string, reference: ChainableReference, lastValue: string, namespace?: string) { + constructor({ name }: { name: InternedString }) { super(); - this.element = element; this.name = name; - this.reference = reference; - this.lastValue = lastValue; - this.namespace = namespace; } - evaluate(vm: UpdatingVM) { - let value = this.reference.value(); - - if (value !== this.lastValue) { - if (this.namespace) { - vm.dom.setAttributeNS(this.element, this.name, value, this.namespace); - } else { - vm.dom.setAttribute(this.element, this.name, value); - } - - this.lastValue = value; - } + evaluate(vm: VM) { + let { name } = this; + let reference = vm.frame.getOperand(); + vm.stack().setAttribute(name, reference); } toJSON(): OpcodeJSON { - let { _guid: guid, type, name, lastValue, namespace } = this; + let { _guid: guid, type, name } = this; let details = dict(); details["name"] = JSON.stringify(name); - details["lastValue"] = JSON.stringify(lastValue); - - if (namespace) { - details["namespace"] = JSON.stringify(namespace); - } + details["value"] = "$OPERAND"; return { guid, type, details }; } @@ -206,13 +327,8 @@ export class DynamicPropOpcode extends Opcode { evaluate(vm: VM) { let { name } = this; - let element = vm.stack().element; let reference = vm.frame.getOperand(); - let value = reference.value(); - - element[name] = value; - - vm.updateWith(new UpdatePropertyOpcode(element, name, reference, value)); + vm.stack().setProperty(name, reference); } toJSON(): OpcodeJSON { @@ -227,44 +343,22 @@ export class DynamicPropOpcode extends Opcode { } } -export class UpdatePropertyOpcode extends DOMUpdatingOpcode { - public type = "update-property"; +export class PatchElementOpcode extends DOMUpdatingOpcode { + public type = "patch-element"; private element: Element; - private name: string; - private reference: ChainableReference; + private attribute: ElementPatchOperation; private lastValue: any; - constructor(element: Element, name: string, reference: ChainableReference, lastValue: any) { + constructor(element: Element, attribute: ElementPatchOperation, lastValue: any) { super(); this.element = element; - this.name = name; - this.reference = reference; + this.attribute = attribute; this.lastValue = lastValue; } evaluate(vm: UpdatingVM) { - let value = this.reference.value(); - - if (value !== this.lastValue) { - this.lastValue = this.element[this.name] = value; - } - } -} - -export class AddClassOpcode extends Opcode { - public type = "add-class"; - - evaluate(vm: VM) { - vm.stack().addClass(vm.frame.getOperand()); - } - - toJSON(): OpcodeJSON { - return { - guid: this._guid, - type: this.type, - args: ["$OPERAND"] - }; + this.lastValue = this.attribute.apply(vm.env.getDOM(), this.element, this.lastValue); } } diff --git a/packages/node_modules/glimmer-runtime/lib/dom.ts b/packages/node_modules/glimmer-runtime/lib/dom.ts index 65c2eeeb21..77d22a3572 100644 --- a/packages/node_modules/glimmer-runtime/lib/dom.ts +++ b/packages/node_modules/glimmer-runtime/lib/dom.ts @@ -20,6 +20,10 @@ export default class DOMHelper { element.setAttributeNS(namespace, name, value); } + setProperty(element: Element, name: string, value: any) { + element[name] = value; + } + removeAttribute(element: Element, name: string) { element.removeAttribute(name); } @@ -76,13 +80,9 @@ export default class DOMHelper { } } -/* tslint:disable:no-unused-variable */ - // http://www.w3.org/TR/html/syntax.html#html-integration-point const SVG_INTEGRATION_POINTS = { foreignObject: 1, desc: 1, title: 1 }; -/* tslint:enable:no-unused-variable */ - // http://www.w3.org/TR/html/syntax.html#adjust-svg-attributes // TODO: Adjust SVG attributes diff --git a/packages/node_modules/glimmer-runtime/lib/syntax/core.ts b/packages/node_modules/glimmer-runtime/lib/syntax/core.ts index 32a79abe2c..7d1bd27adc 100644 --- a/packages/node_modules/glimmer-runtime/lib/syntax/core.ts +++ b/packages/node_modules/glimmer-runtime/lib/syntax/core.ts @@ -79,7 +79,6 @@ import { StaticAttrOpcode, DynamicAttrOpcode, DynamicPropOpcode, - AddClassOpcode, CommentOpcode } from '../compiled/opcodes/dom'; @@ -434,46 +433,6 @@ export class DynamicAttr extends AttributeSyntax { } } -export class AddClass extends AttributeSyntax { - "e1185d30-7cac-4b12-b26a-35327d905d92" = true; - type = "add-class"; - - static fromSpec(node: SerializedStatements.AddClass): AddClass { - let [, value] = node; - - return new AddClass({ value: buildExpression(value) }); - } - - static build(value: ExpressionSyntax): AddClass { - return new this({ value }); - } - - public name = "class"; - public value: ExpressionSyntax; - - constructor({ value }: { value: ExpressionSyntax }) { - super(); - this.value = value; - } - - prettyPrint(): PrettyPrint { - return new PrettyPrint('attr', 'attr', ['class', this.value.prettyPrint()]); - } - - compile(compiler: CompileInto, env: Environment) { - compiler.append(new PutValueOpcode({ expression: this.value.compile(compiler, env) })); - compiler.append(new AddClassOpcode()); - } - - valueSyntax(): ExpressionSyntax { - return this.value; - } - - isAttribute() { - return true; - } -} - export class CloseElement extends StatementSyntax { type = "close-element"; diff --git a/packages/node_modules/glimmer-runtime/lib/syntax/statements.ts b/packages/node_modules/glimmer-runtime/lib/syntax/statements.ts index 2de1b11171..9b79ff1aaa 100644 --- a/packages/node_modules/glimmer-runtime/lib/syntax/statements.ts +++ b/packages/node_modules/glimmer-runtime/lib/syntax/statements.ts @@ -4,7 +4,6 @@ import { Append, DynamicAttr, DynamicProp, - AddClass, Text, Comment, OpenElement, @@ -25,7 +24,6 @@ const { isAppend, isDynamicAttr, isDynamicProp, - isAddClass, isText, isComment, isOpenElement, @@ -39,7 +37,6 @@ export default function(sexp: SerializedStatement, blocks: CompiledInlineBlock[] if (isAppend(sexp)) return Append.fromSpec(sexp); if (isDynamicAttr(sexp)) return DynamicAttr.fromSpec(sexp); if (isDynamicProp(sexp)) return DynamicProp.fromSpec(sexp); - if (isAddClass(sexp)) return AddClass.fromSpec(sexp); if (isText(sexp)) return Text.fromSpec(sexp); if (isComment(sexp)) return Comment.fromSpec(sexp); if (isOpenElement(sexp)) return OpenElement.fromSpec(sexp); diff --git a/packages/node_modules/glimmer-runtime/tests/ember-component-test.ts b/packages/node_modules/glimmer-runtime/tests/ember-component-test.ts index d12099fc21..ec7af3866b 100644 --- a/packages/node_modules/glimmer-runtime/tests/ember-component-test.ts +++ b/packages/node_modules/glimmer-runtime/tests/ember-component-test.ts @@ -305,6 +305,7 @@ function testComponent(title: string, { kind, layout, invokeAs = {}, expected, s assertExpected('aside', expected, attrs); updates.forEach(update => { + ok(true, `Updating with ${JSON.stringify(update)}`); view.rerender(update.context); assertExpected('aside', update.expected, attrs); }); @@ -1174,11 +1175,11 @@ QUnit.test('emberish component should have unique IDs', assert => { module("Glimmer Component - shadowing"); -testComponent('shadowing: non-block with properties on attrs', { +testComponent('shadowing: normal outer attributes are reflected', { kind: 'glimmer', layout: 'In layout - someProp: {{@someProp}}', invokeAs: { attrs: { someProp: 'something here' } }, - expected: 'In layout - someProp: something here' + expected: { attrs: { someProp: 'something here' }, content: 'In layout - someProp: something here' } }); testComponent('shadowing - normal outer attributes clobber inner attributes', { @@ -1188,6 +1189,73 @@ testComponent('shadowing - normal outer attributes clobber inner attributes', { expected: { attrs: { 'data-name': 'Godhuda', 'data-foo': 'foo', 'data-bar': 'bar' }, content: '' } }); +testComponent('shadowing: outer attributes with concat are reflected', { + kind: 'glimmer', + layout: 'In layout - someProp: {{@someProp}}', + invokeAs: { + context: { someProp: 'something here' }, + attrs: { someProp: '{{someProp}}' } + }, + expected: { attrs: { someProp: 'something here' }, content: 'In layout - someProp: something here' }, + updates: [{ + expected: { attrs: { someProp: 'something here' }, content: 'In layout - someProp: something here' } + }, { + context: { someProp: 'something else' }, + expected: { attrs: { someProp: 'something else' }, content: 'In layout - someProp: something else' } + }, { + context: { someProp: '' }, + expected: { attrs: { someProp: '' }, content: 'In layout - someProp: ' } + }, { + context: { someProp: 'something here' }, + expected: { attrs: { someProp: 'something here' }, content: 'In layout - someProp: something here' } + }] +}); + +testComponent('shadowing: outer attributes with concat clobber inner attributes', { + kind: 'glimmer', + layout: { attrs: { 'data-name': 'Godfrey', 'data-foo': 'foo' } }, + invokeAs: { + context: { name: 'Godhuda', foo: 'foo' }, + attrs: { 'data-name': '{{name}}', 'data-foo': '{{foo}}-bar' } + }, + expected: { attrs: { 'data-name': 'Godhuda', 'data-foo': 'foo-bar' }, content: '' }, + updates: [{ + expected: { attrs: { 'data-name': 'Godhuda', 'data-foo': 'foo-bar' }, content: '' } + }, { + context: { name: 'Yehuda', foo: 'baz' }, + expected: { attrs: { 'data-name': 'Yehuda', 'data-foo': 'baz-bar' }, content: '' } + }, { + context: { name: '', foo: '' }, + expected: { attrs: { 'data-name': '', 'data-foo': '-bar' }, content: '' } + }, { + context: { name: 'Godhuda', foo: 'foo' }, + expected: { attrs: { 'data-name': 'Godhuda', 'data-foo': 'foo-bar' }, content: '' } + }] +}); + +testComponent('shadowing: outer attributes clobber inner attributes with concat', { + kind: 'glimmer', + layout: { attrs: { 'data-name': '{{@name}}', 'data-foo': '{{@foo}}-bar' } }, + invokeAs: { + context: { name: 'Godfrey', foo: 'foo' }, + props: { name: 'name', foo: 'foo' }, + attrs: { 'data-name': 'Godhuda', 'data-foo': 'foo-bar' } + }, + expected: { attrs: { 'data-name': 'Godhuda', 'data-foo': 'foo-bar' }, content: '' }, + updates: [{ + expected: { attrs: { 'data-name': 'Godhuda', 'data-foo': 'foo-bar' }, content: '' } + }, { + context: { name: 'Yehuda', foo: 'baz' }, + expected: { attrs: { 'data-name': 'Godhuda', 'data-foo': 'foo-bar' }, content: '' } + }, { + context: { name: '', foo: '' }, + expected: { attrs: { 'data-name': 'Godhuda', 'data-foo': 'foo-bar' }, content: '' } + }, { + context: { name: 'Godhuda', foo: 'foo' }, + expected: { attrs: { 'data-name': 'Godhuda', 'data-foo': 'foo-bar' }, content: '' } + }] +}); + module("Glimmer Component"); let styles = [{ diff --git a/packages/node_modules/glimmer-test-helpers/lib/environment.ts b/packages/node_modules/glimmer-test-helpers/lib/environment.ts index 71f91934f9..e0484b65a9 100644 --- a/packages/node_modules/glimmer-test-helpers/lib/environment.ts +++ b/packages/node_modules/glimmer-test-helpers/lib/environment.ts @@ -63,7 +63,6 @@ import { StaticAttr, DynamicAttr, ValueSyntax, - AddClass, RefSyntax, GetNamedParameterSyntax, @@ -460,8 +459,6 @@ class BasicComponentDefinition extends GenericComponentDefinition(); - attrs.append(new AddClass({ value: EMBER_VIEW })); + attrs.append(new StaticAttr({ name: 'class' as InternedString, value: 'ember-view' as InternedString })); attrs.append(new DynamicAttr({ name: 'id' as InternedString, value: new EmberID() })); return { tag, attrs, body }; @@ -511,7 +508,7 @@ class EmberishGlimmerComponentDefinition extends GenericComponentDefinition('block'); export const isOpenElement = is('openElement'); export const isCloseElement = is('closeElement'); - export const isAddClass = is('addClass'); export const isStaticAttr = is('staticAttr'); export const isDynamicAttr = is('dynamicAttr'); export const isDynamicProp = is('dynamicProp'); @@ -115,7 +113,6 @@ export namespace Statements { | Block | OpenElement | CloseElement - | AddClass | StaticAttr | DynamicAttr | DynamicProp