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

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

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<null>
+    insertBefore: Maybe<SimpleNode>
   ): Option<RemoteLiveBlock>;
   popRemoteElement(): void;
   popElement(): void;

packages/@glimmer/node/lib/serialize-builder.ts

@@ -1,4 +1,11 @@
-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 +103,7 @@ class SerializeBuilder extends NewElementBuilder implements ElementBuilder {
   pushRemoteElement(
     element: SimpleElement,
     cursorId: string,
-    insertBefore: Option<null> = null
+    insertBefore: Maybe<SimpleNode> = null
   ): Option<RemoteLiveBlock> {
     let { dom } = this;
     let script = dom.createElement('script');

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<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);

packages/@glimmer/runtime/lib/vm/element-builder.ts

@@ -13,7 +13,16 @@ 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,15 +218,15 @@ export class NewElementBuilder implements ElementBuilder {
   pushRemoteElement(
     element: SimpleElement,
     guid: string,
-    insertBefore: Option<null>
+    insertBefore: Maybe<SimpleNode>
   ): Option<RemoteLiveBlock> {
     return this.__pushRemoteElement(element, guid, insertBefore);
   }
 
   __pushRemoteElement(
     element: SimpleElement,
     _guid: string,
-    insertBefore: Option<null>
+    insertBefore: Maybe<SimpleNode>
   ): Option<RemoteLiveBlock> {
     this.pushElement(element, insertBefore);
 
@@ -237,7 +246,7 @@ export class NewElementBuilder implements ElementBuilder {
     this.popElement();
   }
 
-  protected pushElement(element: SimpleElement, nextSibling: Option<SimpleNode>) {
+  protected pushElement(element: SimpleElement, nextSibling: Maybe<SimpleNode> = null) {
     this[CURSOR_STACK].push(new CursorImpl(element, nextSibling));
   }
 

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<SimpleNode>) {
+  pushElement(element: SimpleElement, nextSibling: Maybe<SimpleNode> = 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<null>
+    insertBefore: Maybe<SimpleNode>
   ): Option<RemoteLiveBlock> {
     let marker = this.getMarker(element as HTMLElement, cursorId);
 

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;
     }
   });