Modifying in-element semantics to support non-null insertBefore elements

- This is a slight modification of the changes introduced in PR glimmerjs#918, to continue to allow passing non null values to the insertBefore param of the in-element helper
chiragpat · Apr 10, 2019 · a3917d6 · a3917d6
1 parent 99c6410
commit a3917d6
Show file tree

Hide file tree

Showing 8 changed files with 48 additions and 22 deletions.
diff --git 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, '<b>Hello</b><em>there!</em>');
 
-    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', {}, '<b>Hello</b>[Yippie!]<em>there!</em>');
+    this.assertHTML('<!---->');
+    this.assertStableRerender();
+
+    this.rerender({ foo: 'Double Yips!' });
+    equalsElement(externalElement, 'div', {}, '<b>Hello</b>[Double Yips!]<em>there!</em>');
+    this.assertHTML('<!---->');
+    this.assertStableNodes();
+
+    this.rerender({ insertBefore: null });
+    equalsElement(externalElement, 'div', {}, '<b>Hello</b><em>there!</em>[Double Yips!]');
+    this.assertHTML('<!---->');
+    this.assertStableRerender();
+
+    this.rerender({ externalElement: null });
+    equalsElement(externalElement, 'div', {}, '<b>Hello</b><em>there!</em>');
+    this.assertHTML('<!---->');
+    this.assertStableRerender();
+
+    this.rerender({ externalElement, insertBefore: externalElement.lastChild, foo: 'Yippie!' });
+    equalsElement(externalElement, 'div', {}, '<b>Hello</b>[Yippie!]<em>there!</em>');
+    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
@@ -41,7 +41,7 @@ export interface DOMStack {
   pushRemoteElement(
     element: SimpleElement,
     guid: string,
-    insertBefore: Option<null>
+    insertBefore: Option<SimpleNode>
   ): Option<RemoteLiveBlock>;
   popRemoteElement(): void;
   popElement(): void;

diff --git 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> = null
+    insertBefore: Option<SimpleNode> = null
   ): Option<RemoteLiveBlock> {
     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
@@ -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
@@ -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<SimpleNode>;
   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<null>;
+  if (insertBeforeRef.value() !== undefined) {
+    if (isConst(insertBeforeRef)) {
+      insertBefore = check(insertBeforeRef.value(), CheckOption(CheckNode));
+    } else {
+      let cache = new ReferenceCache(insertBeforeRef as Reference<Option<SimpleNode>>);
+      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
@@ -209,15 +209,15 @@ export class NewElementBuilder implements ElementBuilder {
   pushRemoteElement(
     element: SimpleElement,
     guid: string,
-    insertBefore: Option<null>
+    insertBefore: Option<SimpleNode>
   ): Option<RemoteLiveBlock> {
     return this.__pushRemoteElement(element, guid, insertBefore);
   }
 
   __pushRemoteElement(
     element: SimpleElement,
     _guid: string,
-    insertBefore: Option<null>
+    insertBefore: Option<SimpleNode>
   ): Option<RemoteLiveBlock> {
     this.pushElement(element, insertBefore);
 

diff --git 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<null>
+    insertBefore: Option<SimpleNode>
   ): Option<RemoteLiveBlock> {
     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
@@ -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;
     }
   });