[BUGFIX beta] Avoid breaking {{-in-element}} usage

The intimate `{{#-in-element}}` API previously defaults to appending to the remote element without clearing. With the VM upgrade, the default has changed to clearing the remote element as specified by the RFC. Since we are not ready to promote the public API yet, we should preserve the exisiting behavior and let addon authors migrate to the new public API directly when it is ready.
emberjs · Feb 4, 2020 · da6c5fa · da6c5fa
1 parent e3b272e
commit da6c5fa
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 15 deletions.
diff --git a/packages/@ember/-internals/glimmer/tests/integration/syntax/in-element-test.js b/packages/@ember/-internals/glimmer/tests/integration/syntax/in-element-test.js
@@ -47,8 +47,41 @@ moduleFor(
       equalTokens(someElement, 'Whoop!');
     }
 
-    ['@test allows insertBefore=null']() {
+    ['@test it appends to the extenal element by default']() {
       let someElement = document.createElement('div');
+      someElement.appendChild(document.createTextNode('foo '));
+
+      this.render(
+        strip`
+          {{#-in-element someElement}}
+            {{text}}
+          {{/-in-element}}
+        `,
+        {
+          someElement,
+          text: 'bar',
+        }
+      );
+
+      equalTokens(this.element, '<!---->');
+      equalTokens(someElement, 'foo bar');
+
+      this.assertStableRerender();
+
+      runTask(() => set(this.context, 'text', 'bar!!'));
+
+      equalTokens(this.element, '<!---->');
+      equalTokens(someElement, 'foo bar!!');
+
+      runTask(() => set(this.context, 'text', 'bar'));
+
+      equalTokens(this.element, '<!---->');
+      equalTokens(someElement, 'foo bar');
+    }
+
+    ['@test allows appending to the external element with insertBefore=null']() {
+      let someElement = document.createElement('div');
+      someElement.appendChild(document.createTextNode('foo '));
 
       this.render(
         strip`
@@ -58,24 +91,56 @@ moduleFor(
         `,
         {
           someElement,
-          text: 'Whoop!',
+          text: 'bar',
         }
       );
 
       equalTokens(this.element, '<!---->');
-      equalTokens(someElement, 'Whoop!');
+      equalTokens(someElement, 'foo bar');
 
       this.assertStableRerender();
 
-      runTask(() => set(this.context, 'text', 'Huzzah!!'));
+      runTask(() => set(this.context, 'text', 'bar!!'));
 
       equalTokens(this.element, '<!---->');
-      equalTokens(someElement, 'Huzzah!!');
+      equalTokens(someElement, 'foo bar!!');
 
-      runTask(() => set(this.context, 'text', 'Whoop!'));
+      runTask(() => set(this.context, 'text', 'bar'));
 
       equalTokens(this.element, '<!---->');
-      equalTokens(someElement, 'Whoop!');
+      equalTokens(someElement, 'foo bar');
+    }
+
+    ['@test allows clearing the external element with insertBefore=undefined']() {
+      let someElement = document.createElement('div');
+      someElement.appendChild(document.createTextNode('foo '));
+
+      this.render(
+        strip`
+          {{#-in-element someElement insertBefore=undefined}}
+            {{text}}
+          {{/-in-element}}
+        `,
+        {
+          someElement,
+          text: 'bar',
+        }
+      );
+
+      equalTokens(this.element, '<!---->');
+      equalTokens(someElement, 'bar');
+
+      this.assertStableRerender();
+
+      runTask(() => set(this.context, 'text', 'bar!!'));
+
+      equalTokens(this.element, '<!---->');
+      equalTokens(someElement, 'bar!!');
+
+      runTask(() => set(this.context, 'text', 'bar'));
+
+      equalTokens(this.element, '<!---->');
+      equalTokens(someElement, 'bar');
     }
 
     ['@test does not allow insertBefore=non-null-value']() {
@@ -93,7 +158,7 @@ moduleFor(
             text: 'Whoop!',
           }
         );
-      }, /Can only pass a null literal to insertBefore in -in-element, received:/);
+      }, /Can only pass a null or undefined literals to insertBefore in -in-element, received:/);
     }
 
     ['@test components are cleaned up properly'](assert) {

diff --git a/packages/ember-template-compiler/lib/plugins/transform-in-element.ts b/packages/ember-template-compiler/lib/plugins/transform-in-element.ts
@@ -66,28 +66,29 @@ export default function transformInElement(env: ASTPluginEnvironment): ASTPlugin
 
           // replicate special hash arguments added here:
           // https://github.com/glimmerjs/glimmer-vm/blob/ba9b37d44b85fa1385eeeea71910ff5798198c8e/packages/%40glimmer/syntax/lib/parser/handlebars-node-visitors.ts#L340-L363
-          let hasInsertBefore = false;
+          let needsInsertBefore = true;
           let hash = node.hash;
           hash.pairs.forEach(pair => {
             if (pair.key === 'insertBefore') {
               assert(
-                `Can only pass a null literal to insertBefore in -in-element, received: ${JSON.stringify(
+                `Can only pass a null or undefined literals to insertBefore in -in-element, received: ${JSON.stringify(
                   pair.value
                 )}`,
-                pair.value.type === 'NullLiteral'
+                pair.value.type === 'NullLiteral' || pair.value.type === 'UndefinedLiteral'
               );
 
-              hasInsertBefore = true;
+              needsInsertBefore = false;
             }
           });
 
           let guid = b.literal('StringLiteral', `%cursor:${cursorCount++}%`);
           let guidPair = b.pair('guid', guid);
           hash.pairs.unshift(guidPair);
 
-          if (!hasInsertBefore) {
-            let undefinedLiteral = b.literal('UndefinedLiteral', undefined);
-            let nextSibling = b.pair('insertBefore', undefinedLiteral);
+          // Maintain compatibility with previous -in-element behavior (defaults to append, not clear)
+          if (needsInsertBefore) {
+            let nullLiteral = b.literal('NullLiteral', null);
+            let nextSibling = b.pair('insertBefore', nullLiteral);
             hash.pairs.push(nextSibling);
           }
         }