fix(runtime): address child reconciliation issues with automatic keys

src/compiler/transformers/automatic-key-insertion/automatic-key-insertion.spec.ts

@@ -0,0 +1,88 @@
+import { transpileModule } from '../test/transpile';
+import { formatCode } from '../test/utils';
+import { performAutomaticKeyInsertion } from '.';
+
+function transpile(code: string) {
+  return transpileModule(code, null, null, [performAutomaticKeyInsertion]);
+}
+
+describe('convert-decorators', () => {
+  it('should add a key to one JSX opener', async () => {
+    const t = transpile(`
+    @Component({tag: 'cmp-a'})
+      export class CmpA {
+        render() {
+          return <div>test</div>
+      }
+    `);
+
+    console.log(await formatCode(t.outputText));
+  });
+
+  it('should add it to all of them!', async () => {
+    const t = transpile(`
+@Component({
+  tag: 'scoped-conditional',
+  scoped: true,
+})
+export class ScopedConditional {
+  @Prop() renderHello: boolean = false;
+
+  render() {
+    return (
+      <div>
+        {/* prior to fixing the bug */}
+        {/* - if you remove the conditional below, it works */}
+        {/* - if you remove the <div /> around .tag, it works */}
+        {/* - if you add additional elements between the conditional and the second <div/>, it works */}
+
+        {/* Note: Need the conditional's first half, _and_ the innerHTML attr */}
+        {/* Interestingly, if we replace innerHTML with a text node as a child of the <div>, */}
+        {/* we get a separate error where the slot doesn't get put in the correct place */}
+        {this.renderHello && <div class="tag" innerHTML={'Hello'} />}
+        {/* This div below must be there too */}
+        <div>
+          before slot-&gt;
+          <slot key="my-slot" />
+          &lt;-after slot
+        </div>
+      </div>
+    );
+  }
+}
+    `);
+
+    console.log(await formatCode(t.outputText));
+  });
+
+  it('svg huh?', async () => {
+    const t = transpile(`
+      @Component({
+        tag: 'svg-attr',
+      })
+      class SvgAttr {
+        @Prop() isOpen = false;
+
+        render() {
+          return (
+            <div>
+              <div>
+                {this.isOpen ? (
+                  <svg viewBox="0 0 54 54">
+                    <rect transform="rotate(45 27 27)" y="22" width="54" height="10" rx="2" />
+                  </svg>
+                ) : (
+                  <svg viewBox="0 0 54 54">
+                    <rect y="0" width="54" height="10" rx="2" />
+                  </svg>
+                )}
+              </div>
+            </div>
+          );
+        }
+      }
+    `);
+
+    console.log(await formatCode(t.outputText));
+  });
+});

src/compiler/transformers/automatic-key-insertion/index.ts

@@ -0,0 +1,158 @@
+import { createHash } from 'crypto';
+import ts from 'typescript';
+
+/**
+ * A transformer factory to create a transformer which will add `key`
+ * properties to all of the JSX nodes contained inside of a component's `render`
+ * function.
+ *
+ * This can be thought of as transforming the following:
+ *
+ * ```tsx
+ * class MyComponent {
+ *   render() {
+ *     <div>hey!</div>
+ *   }
+ * }
+ * ```
+ *
+ * to this:
+ *
+ * ```tsx
+ * class MyComponent {
+ *   render() {
+ *     <div key="a-unique-key">hey!</div>
+ *   }
+ * }
+ * ```
+ *
+ * The keys added are generated by {@link deriveJSXKey}.
+ *
+ * @param transformCtx a transformation context
+ * @returns a typescript transformer for inserting keys into JSX nodes
+ */
+export const performAutomaticKeyInsertion = (transformCtx: ts.TransformationContext): ts.Transformer<ts.SourceFile> => {
+  /**
+   * This is our outer-most visitor function which serves to locate a class
+   * declaration, at which point it hands things over to the next visitor
+   * function ({@link findRenderMethodVisitor}) which locates the `render`
+   * method.
+   *
+   * @param node a typescript syntax tree node
+   */
+  function findClassDeclVisitor(node: ts.Node): ts.VisitResult<ts.Node> {
+    if (ts.isClassDeclaration(node)) {
+      return ts.visitEachChild(node, findRenderMethodVisitor, transformCtx);
+    } else {
+      return ts.visitEachChild(node, findClassDeclVisitor, transformCtx);
+    }
+  }
+
+  /**
+   * This middle visitor function is responsible for finding the render method
+   * on a Stencil class and then passing off responsibility to the inner-most
+   * visitor, which deals with syntax nodes inside the method.
+   *
+   * @param node a typescript syntax tree node
+   */
+  function findRenderMethodVisitor(node: ts.Node): ts.VisitResult<ts.Node> {
+    if (ts.isMethodDeclaration(node) && node.name.getText() === 'render') {
+      return ts.visitEachChild(node, jsxElementVisitor, transformCtx);
+    } else {
+      return ts.visitEachChild(node, findRenderMethodVisitor, transformCtx);
+    }
+  }
+
+  /**
+   * Our inner-most visitor function. This will edit any JSX nodes that it
+   * finds, adding a `key` attribute to them via {@link addKeyAttr}.
+   *
+   * @param node a typescript syntax tree node
+   */
+  function jsxElementVisitor(node: ts.Node): ts.VisitResult<ts.Node> {
+    if (isJSXElWithAttrs(node)) {
+      return addKeyAttr(node);
+    } else {
+      return ts.visitEachChild(node, jsxElementVisitor, transformCtx);
+    }
+  }
+
+  return (tsSourceFile) => {
+    return ts.visitEachChild(tsSourceFile, findClassDeclVisitor, transformCtx);
+  };
+};
+
+/**
+ * An incrementing-number generator, just as a little extra 'uniqueness'
+ * insurance for {@link deriveJSXKey}
+ */
+const incrementer = (function* () {
+  let val = 0;
+  while (true) {
+    yield val++;
+  }
+})();
+
+/**
+ * Generate a unique key for a given JSX element. The key is creating by
+ * concatenating and then hashing (w/ SHA1) the following:
+ *
+ * - an incrementing value
+ * - the element's tag name
+ * - the start position for the element's token in the original source file
+ * - the end position for the element's token in the original source file
+ *
+ * It is hoped this provides enough uniqueness that a collision won't occur.
+ *
+ * @param jsxElement a typescript JSX syntax tree node which needs a key
+ * @returns a computed unique key for that element
+ */
+function deriveJSXKey(jsxElement: ts.JsxOpeningElement | ts.JsxSelfClosingElement): string {
+  const hash = createHash('sha1')
+    .update(`${incrementer.next().value}__${jsxElement.tagName}__${jsxElement.pos}_${jsxElement.end}`)
+    .digest('hex')
+    .toLowerCase();
+  return hash;
+}
+
+/**
+ * description
+ *
+ * @param node a typescript syntax tree node
+ */
+function isJSXElWithAttrs(node: ts.Node): node is ts.JsxOpeningElement | ts.JsxSelfClosingElement {
+  return ts.isJsxOpeningElement(node) || ts.isJsxSelfClosingElement(node);
+}
+
+/**
+ * Given a JSX syntax tree node update it to include a unique key attribute.
+ *
+ * @param jsxElement a typescript JSX syntax tree node
+ */
+function addKeyAttr(
+  jsxElement: ts.JsxOpeningElement | ts.JsxSelfClosingElement,
+): ts.JsxOpeningElement | ts.JsxSelfClosingElement {
+  const attrs = ts.factory.createJsxAttributes([
+    ts.factory.createJsxAttribute(
+      ts.factory.createIdentifier('key'),
+      ts.factory.createStringLiteral(deriveJSXKey(jsxElement)),
+    ),
+    ...jsxElement.attributes.properties,
+  ]);
+
+  if (jsxElement.tagName.getText() === 'rect' || jsxElement.tagName.getText() === 'svg') {
+    // TODO diagnose the issue with svg tags
+    return jsxElement;
+  }
+
+  if (jsxElement.tagName.getText() === '') {
+    // TODO do we need this check?
+    return jsxElement;
+  }
+
+  if (ts.isJsxOpeningElement(jsxElement)) {
+    return ts.factory.updateJsxOpeningElement(jsxElement, jsxElement.tagName, jsxElement.typeArguments, attrs);
+  } else {
+    return ts.factory.updateJsxSelfClosingElement(jsxElement, jsxElement.tagName, jsxElement.typeArguments, attrs);
+  }
+}

src/compiler/transpile/run-program.ts

@@ -12,6 +12,7 @@ import ts from 'typescript';
 import type * as d from '../../declarations';
 import { updateComponentBuildConditionals } from '../app-core/app-data';
 import { resolveComponentDependencies } from '../entries/resolve-component-dependencies';
+import { performAutomaticKeyInsertion } from '../transformers/automatic-key-insertion';
 import { convertDecoratorsToStatic } from '../transformers/decorators-to-static/convert-decorators';
 import { rewriteAliasedDTSImportPaths } from '../transformers/rewrite-aliased-paths';
 import { updateModule } from '../transformers/static-to-meta/parse-static';
@@ -66,7 +67,10 @@ export const runTsProgram = async (
   };
 
   const transformers: ts.CustomTransformers = {
-    before: [convertDecoratorsToStatic(config, buildCtx.diagnostics, tsTypeChecker, tsProgram)],
+    before: [
+      convertDecoratorsToStatic(config, buildCtx.diagnostics, tsTypeChecker, tsProgram),
+      performAutomaticKeyInsertion,
+    ],
     afterDeclarations: [],
   };
 

src/compiler/transpile/transpile-module.ts

@@ -5,6 +5,7 @@ import ts from 'typescript';
 import type * as d from '../../declarations';
 import { BuildContext } from '../build/build-ctx';
 import { CompilerContext } from '../build/compiler-ctx';
+import { performAutomaticKeyInsertion } from '../transformers/automatic-key-insertion';
 import { lazyComponentTransform } from '../transformers/component-lazy/transform-lazy-component';
 import { nativeComponentTransform } from '../transformers/component-native/tranform-to-native-component';
 import { convertDecoratorsToStatic } from '../transformers/decorators-to-static/convert-decorators';
@@ -113,6 +114,7 @@ export const transpileModule = (
   const transformers: ts.CustomTransformers = {
     before: [
       convertDecoratorsToStatic(config, buildCtx.diagnostics, typeChecker, program),
+      performAutomaticKeyInsertion,
       updateStencilCoreImports(transformOpts.coreImportPath),
     ],
     after: [convertStaticToMeta(config, compilerCtx, buildCtx, typeChecker, null, transformOpts)],