From 069c7410e3bbf33386afd33bfa47ca7565f99be9 Mon Sep 17 00:00:00 2001 From: Chirag Patel Date: Wed, 10 Apr 2019 11:48:33 -0700 Subject: [PATCH] Modifying in-element semantics to support non-null insertBefore elements - This is a slight modification of the changes introduced in PR #918, to continue to allow passing non null values to the insertBefore param of the in-element helper --- .../lib/suites/in-element.ts | 39 ++++++++++++++----- .../interfaces/lib/dom/attributes.d.ts | 4 +- .../@glimmer/node/lib/serialize-builder.ts | 4 +- .../runtime/lib/compiled/opcodes/dom.ts | 17 ++++++-- .../runtime/lib/vm/element-builder.ts | 8 ++-- .../runtime/lib/vm/rehydrate-builder.ts | 6 +-- .../lib/parser/handlebars-node-visitors.ts | 4 -- 7 files changed, 54 insertions(+), 28 deletions(-) diff --git a/packages/@glimmer/integration-tests/lib/suites/in-element.ts b/packages/@glimmer/integration-tests/lib/suites/in-element.ts index 960377f7d7..fd360121af 100644 --- a/packages/@glimmer/integration-tests/lib/suites/in-element.ts +++ b/packages/@glimmer/integration-tests/lib/suites/in-element.ts @@ -131,17 +131,38 @@ export class InElementSuite extends RenderTest { } @test - '`insertBefore` can only be null'() { + 'With insertBefore'() { let externalElement = this.delegate.createElement('div'); - let before = this.delegate.createElement('div'); + replaceHTML(externalElement, 'Hellothere!'); - this.assert.throws(() => { - this.render('{{#in-element externalElement insertBefore=before}}[{{foo}}]{{/in-element}}', { - externalElement, - before, - foo: 'Yippie!', - }); - }, /insertBefore only takes `null` as an argument/); + this.render( + stripTight`{{#in-element externalElement insertBefore=insertBefore}}[{{foo}}]{{/in-element}}`, + { externalElement, insertBefore: externalElement.lastChild, foo: 'Yippie!' } + ); + + equalsElement(externalElement, 'div', {}, 'Hello[Yippie!]there!'); + this.assertHTML(''); + this.assertStableRerender(); + + this.rerender({ foo: 'Double Yips!' }); + equalsElement(externalElement, 'div', {}, 'Hello[Double Yips!]there!'); + this.assertHTML(''); + this.assertStableNodes(); + + this.rerender({ insertBefore: null }); + equalsElement(externalElement, 'div', {}, 'Hellothere![Double Yips!]'); + this.assertHTML(''); + this.assertStableRerender(); + + this.rerender({ externalElement: null }); + equalsElement(externalElement, 'div', {}, 'Hellothere!'); + this.assertHTML(''); + this.assertStableRerender(); + + this.rerender({ externalElement, insertBefore: externalElement.lastChild, foo: 'Yippie!' }); + equalsElement(externalElement, 'div', {}, 'Hello[Yippie!]there!'); + this.assertHTML(''); + this.assertStableRerender(); } @test diff --git a/packages/@glimmer/interfaces/lib/dom/attributes.d.ts b/packages/@glimmer/interfaces/lib/dom/attributes.d.ts index e9afc6432f..53fdc5802f 100644 --- a/packages/@glimmer/interfaces/lib/dom/attributes.d.ts +++ b/packages/@glimmer/interfaces/lib/dom/attributes.d.ts @@ -6,7 +6,7 @@ import { SimpleDocumentFragment, AttrNamespace, } from '@simple-dom/interface'; -import { Option, DestroySymbol, SymbolDestroyable } from '../core'; +import { Option, DestroySymbol, SymbolDestroyable, Maybe } from '../core'; import { Bounds, Cursor } from './bounds'; import { ElementOperations, Environment } from '../runtime'; import { GlimmerTreeConstruction, GlimmerTreeChanges } from './changes'; @@ -41,7 +41,7 @@ export interface DOMStack { pushRemoteElement( element: SimpleElement, guid: string, - insertBefore: Option + insertBefore: Maybe ): Option; popRemoteElement(): void; popElement(): void; diff --git a/packages/@glimmer/node/lib/serialize-builder.ts b/packages/@glimmer/node/lib/serialize-builder.ts index dcec3b2e11..f5e95e2845 100644 --- a/packages/@glimmer/node/lib/serialize-builder.ts +++ b/packages/@glimmer/node/lib/serialize-builder.ts @@ -1,4 +1,4 @@ -import { Bounds, Environment, Option, ElementBuilder, ModifierManager } from '@glimmer/interfaces'; +import { Bounds, Environment, Option, ElementBuilder, ModifierManager, Maybe } from '@glimmer/interfaces'; import { ConcreteBounds, NewElementBuilder } from '@glimmer/runtime'; import { RemoteLiveBlock } from '@glimmer/runtime'; import { SimpleElement, SimpleNode, SimpleText } from '@simple-dom/interface'; @@ -96,7 +96,7 @@ class SerializeBuilder extends NewElementBuilder implements ElementBuilder { pushRemoteElement( element: SimpleElement, cursorId: string, - insertBefore: Option = null + insertBefore: Maybe = null ): Option { let { dom } = this; let script = dom.createElement('script'); diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts index b1ec2374a5..151a222c8b 100644 --- a/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts +++ b/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts @@ -7,7 +7,7 @@ import { isConst, isConstTag, } from '@glimmer/reference'; -import { check, CheckString, CheckElement } from '@glimmer/debug'; +import { check, CheckString, CheckElement, CheckOption, CheckNode } from '@glimmer/debug'; import { Op, Option, ModifierManager } from '@glimmer/interfaces'; import { $t0 } from '@glimmer/vm'; import { @@ -21,8 +21,8 @@ import { Assert } from './vm'; import { DynamicAttribute } from '../../vm/attributes/dynamic'; import { CheckReference, CheckArguments, CheckOperations } from './-debug-strip'; import { CONSTANTS } from '../../symbols'; -import { SimpleElement } from '@simple-dom/interface'; -import { expect } from '@glimmer/util'; +import { SimpleElement, SimpleNode } from '@simple-dom/interface'; +import { expect, Maybe } from '@glimmer/util'; APPEND_OPCODES.add(Op.Text, (vm, { op1: text }) => { vm.elements().appendText(vm[CONSTANTS].getString(text)); @@ -47,6 +47,7 @@ APPEND_OPCODES.add(Op.PushRemoteElement, vm => { let guidRef = check(vm.stack.pop(), CheckReference); let element: SimpleElement; + let insertBefore: Maybe; let guid = guidRef.value() as string; if (isConst(elementRef)) { @@ -57,7 +58,15 @@ APPEND_OPCODES.add(Op.PushRemoteElement, vm => { vm.updateWith(new Assert(cache)); } - let insertBefore = insertBeforeRef.value() as Option; + if (insertBeforeRef.value() !== undefined) { + if (isConst(insertBeforeRef)) { + insertBefore = check(insertBeforeRef.value(), CheckOption(CheckNode)); + } else { + let cache = new ReferenceCache(insertBeforeRef as Reference>); + insertBefore = check(cache.peek(), CheckOption(CheckNode)); + vm.updateWith(new Assert(cache)); + } + } let block = vm.elements().pushRemoteElement(element, guid, insertBefore); if (block) vm.associateDestroyable(block); diff --git a/packages/@glimmer/runtime/lib/vm/element-builder.ts b/packages/@glimmer/runtime/lib/vm/element-builder.ts index c9f575982f..a156fca8aa 100644 --- a/packages/@glimmer/runtime/lib/vm/element-builder.ts +++ b/packages/@glimmer/runtime/lib/vm/element-builder.ts @@ -13,7 +13,7 @@ import { Cursor, ModifierManager, } from '@glimmer/interfaces'; -import { assert, DESTROY, expect, LinkedList, LinkedListNode, Option, Stack } from '@glimmer/util'; +import { assert, DESTROY, expect, LinkedList, LinkedListNode, Option, Stack, Maybe } from '@glimmer/util'; import { AttrNamespace, SimpleComment, @@ -209,7 +209,7 @@ export class NewElementBuilder implements ElementBuilder { pushRemoteElement( element: SimpleElement, guid: string, - insertBefore: Option + insertBefore: Maybe ): Option { return this.__pushRemoteElement(element, guid, insertBefore); } @@ -217,7 +217,7 @@ export class NewElementBuilder implements ElementBuilder { __pushRemoteElement( element: SimpleElement, _guid: string, - insertBefore: Option + insertBefore: Maybe ): Option { this.pushElement(element, insertBefore); @@ -237,7 +237,7 @@ export class NewElementBuilder implements ElementBuilder { this.popElement(); } - protected pushElement(element: SimpleElement, nextSibling: Option) { + protected pushElement(element: SimpleElement, nextSibling: Maybe = null) { this[CURSOR_STACK].push(new CursorImpl(element, nextSibling)); } diff --git a/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts b/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts index c5ba914f35..caefc07072 100644 --- a/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts +++ b/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts @@ -1,5 +1,5 @@ import { Bounds, Environment, Option, ElementBuilder } from '@glimmer/interfaces'; -import { assert, expect, Stack } from '@glimmer/util'; +import { assert, expect, Stack, Maybe } from '@glimmer/util'; import { AttrNamespace, Namespace, @@ -75,7 +75,7 @@ export class RehydrateBuilder extends NewElementBuilder implements ElementBuilde this.currentCursor!.candidate = node; } - pushElement(element: SimpleElement, nextSibling: Option) { + pushElement(element: SimpleElement, nextSibling: Maybe = null) { let { blockDepth = 0 } = this; let cursor = new RehydratingCursor(element, nextSibling, blockDepth); let currentCursor = this.currentCursor; @@ -379,7 +379,7 @@ export class RehydrateBuilder extends NewElementBuilder implements ElementBuilde __pushRemoteElement( element: SimpleElement, cursorId: string, - insertBefore: Option + insertBefore: Maybe ): Option { let marker = this.getMarker(element as HTMLElement, cursorId); diff --git a/packages/@glimmer/syntax/lib/parser/handlebars-node-visitors.ts b/packages/@glimmer/syntax/lib/parser/handlebars-node-visitors.ts index e86b503697..aacfe03426 100644 --- a/packages/@glimmer/syntax/lib/parser/handlebars-node-visitors.ts +++ b/packages/@glimmer/syntax/lib/parser/handlebars-node-visitors.ts @@ -448,10 +448,6 @@ function addInElementHash(cursor: string, hash: AST.Hash, loc: AST.SourceLocatio } if (pair.key === 'insertBefore') { - if (pair.value.type !== 'NullLiteral') { - throw new SyntaxError('insertBefore only takes `null` as an argument', loc); - } - hasInsertBefore = true; } });