From a3917d61aa1614a60b29a4ef57d0550812eb50c5 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 | 2 +- .../@glimmer/node/lib/serialize-builder.ts | 2 +- .../opcode-compiler/lib/syntax/builtins.ts | 2 +- .../runtime/lib/compiled/opcodes/dom.ts | 15 +++++-- .../runtime/lib/vm/element-builder.ts | 4 +- .../runtime/lib/vm/rehydrate-builder.ts | 2 +- .../lib/parser/handlebars-node-visitors.ts | 4 -- 8 files changed, 48 insertions(+), 22 deletions(-) diff --git a/packages/@glimmer/integration-tests/lib/suites/in-element.ts b/packages/@glimmer/integration-tests/lib/suites/in-element.ts index 960377f7d..fd360121a 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 e9afc6432..1e5950f2e 100644 --- a/packages/@glimmer/interfaces/lib/dom/attributes.d.ts +++ b/packages/@glimmer/interfaces/lib/dom/attributes.d.ts @@ -41,7 +41,7 @@ export interface DOMStack { pushRemoteElement( element: SimpleElement, guid: string, - insertBefore: Option + insertBefore: Option ): Option; popRemoteElement(): void; popElement(): void; diff --git a/packages/@glimmer/node/lib/serialize-builder.ts b/packages/@glimmer/node/lib/serialize-builder.ts index dcec3b2e1..1ce6a4a57 100644 --- a/packages/@glimmer/node/lib/serialize-builder.ts +++ b/packages/@glimmer/node/lib/serialize-builder.ts @@ -96,7 +96,7 @@ class SerializeBuilder extends NewElementBuilder implements ElementBuilder { pushRemoteElement( element: SimpleElement, cursorId: string, - insertBefore: Option = null + insertBefore: Option = null ): Option { let { dom } = this; let script = dom.createElement('script'); diff --git a/packages/@glimmer/opcode-compiler/lib/syntax/builtins.ts b/packages/@glimmer/opcode-compiler/lib/syntax/builtins.ts index cf6b1e51b..064485006 100644 --- a/packages/@glimmer/opcode-compiler/lib/syntax/builtins.ts +++ b/packages/@glimmer/opcode-compiler/lib/syntax/builtins.ts @@ -173,7 +173,7 @@ export function populateBuiltins( for (let i = 0; i < keys.length; i++) { let key = keys[i]; - if (key === 'guid' || key === 'insertBefore') { + if (key === 'insertBefore' || key === 'guid') { actions.push(op('Expr', values[i])); } else { throw new Error(`SYNTAX ERROR: #in-element does not take a \`${keys[0]}\` option`); diff --git a/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts b/packages/@glimmer/runtime/lib/compiled/opcodes/dom.ts index b1ec2374a..3314c18b5 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,7 +21,7 @@ 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 { SimpleElement, SimpleNode } from '@simple-dom/interface'; import { expect } from '@glimmer/util'; APPEND_OPCODES.add(Op.Text, (vm, { op1: text }) => { @@ -47,6 +47,7 @@ APPEND_OPCODES.add(Op.PushRemoteElement, vm => { let guidRef = check(vm.stack.pop(), CheckReference); let element: SimpleElement; + let insertBefore: Option; 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 c9f575982..e0671c684 100644 --- a/packages/@glimmer/runtime/lib/vm/element-builder.ts +++ b/packages/@glimmer/runtime/lib/vm/element-builder.ts @@ -209,7 +209,7 @@ export class NewElementBuilder implements ElementBuilder { pushRemoteElement( element: SimpleElement, guid: string, - insertBefore: Option + insertBefore: Option ): Option { return this.__pushRemoteElement(element, guid, insertBefore); } @@ -217,7 +217,7 @@ export class NewElementBuilder implements ElementBuilder { __pushRemoteElement( element: SimpleElement, _guid: string, - insertBefore: Option + insertBefore: Option ): Option { this.pushElement(element, insertBefore); diff --git a/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts b/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts index c5ba914f3..361bf2efd 100644 --- a/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts +++ b/packages/@glimmer/runtime/lib/vm/rehydrate-builder.ts @@ -379,7 +379,7 @@ export class RehydrateBuilder extends NewElementBuilder implements ElementBuilde __pushRemoteElement( element: SimpleElement, cursorId: string, - insertBefore: Option + insertBefore: Option ): 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 e86b50369..aacfe0342 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; } });