diff --git a/src/compiler/transformers/automatic-key-insertion/automatic-key-insertion.spec.ts b/src/compiler/transformers/automatic-key-insertion/automatic-key-insertion.spec.ts index 01384222f78..f0e9ceb0972 100644 --- a/src/compiler/transformers/automatic-key-insertion/automatic-key-insertion.spec.ts +++ b/src/compiler/transformers/automatic-key-insertion/automatic-key-insertion.spec.ts @@ -262,6 +262,105 @@ describe('automatic key insertion', () => { ); }); + it('should add a key to a conditionally-rendered static element', async () => { + jest + .spyOn(keyInsertionUtils, 'deriveJSXKey') + .mockReturnValueOnce('my-best-key') + .mockReturnValueOnce('my-worst-key'); + const t = transpile(` + @Component({tag: 'cmp-a'}) + export class CmpA { + yes = false; + render() { + return ( +
+ { someConditional && inner } +
+ ) + } + }`); + expect(await formatCode(t.outputText)).toBe( + `export class CmpA { + constructor() { + this.yes = false; + } + render() { + return h('div', { key: 'my-best-key' }, someConditional && h('span', { key: 'my-worst-key' }, 'inner')); + } + static get is() { + return 'cmp-a'; + } +} +`, + ); + }); + + it('should not add a key to an IIFE in JSX', async () => { + jest + .spyOn(keyInsertionUtils, 'deriveJSXKey') + .mockReturnValueOnce('my-best-key') + .mockReturnValueOnce('my-worst-key'); + const t = transpile(` + @Component({tag: 'cmp-a'}) + export class CmpA { + yes = false; + render() { + return ( +
+ { (() =>
foo
)() } +
+ ) + } + }`); + expect(await formatCode(t.outputText)).toBe( + `export class CmpA { + constructor() { + this.yes = false; + } + render() { + return h('div', { key: 'my-best-key' }, (() => h('div', null, 'foo'))()); + } + static get is() { + return 'cmp-a'; + } +} +`, + ); + }); + + it('should not add a key within function arguments in JSX', async () => { + jest + .spyOn(keyInsertionUtils, 'deriveJSXKey') + .mockReturnValueOnce('my-best-key') + .mockReturnValueOnce('my-worst-key'); + const t = transpile(` + @Component({tag: 'cmp-a'}) + export class CmpA { + yes = false; + render() { + return ( +
+ { func(
foo
) } +
+ ) + } + }`); + expect(await formatCode(t.outputText)).toBe( + `export class CmpA { + constructor() { + this.yes = false; + } + render() { + return h('div', { key: 'my-worst-key' }, func(h('div', null, 'foo'))); + } + static get is() { + return 'cmp-a'; + } +} +`, + ); + }); + it('should not transform JSX in methods with multiple returns', async () => { jest.spyOn(keyInsertionUtils, 'deriveJSXKey').mockReturnValue('shouldnt-see-key'); const t = transpile(` diff --git a/src/compiler/transformers/automatic-key-insertion/index.ts b/src/compiler/transformers/automatic-key-insertion/index.ts index 8fab80f0505..974bda42394 100644 --- a/src/compiler/transformers/automatic-key-insertion/index.ts +++ b/src/compiler/transformers/automatic-key-insertion/index.ts @@ -71,11 +71,33 @@ export const performAutomaticKeyInsertion = (transformCtx: ts.TransformationCont * @returns the result of handling the node */ function findRenderMethodVisitor(node: ts.Node): ts.VisitResult { - // we want to keep going (to drill down into JSX nodes and transform them) only in particular circumstances: + // we want to keep going (to drill down into JSX nodes and transform them) + // only in particular circumstances: // // 1. the syntax tree node is a method declaration // 2. this method's name is 'render' // 3. the method only has a single return statement + // + // We want to only keep going if there's a single return statement because + // if there are multiple return statements inserting keys could cause + // needless re-renders. If a `render` method looked like this, for + // instance: + // + // ```tsx + // render() { + // if (foo) { + // return
hey!
; + // } else { + // return
hay!
; + // } + // } + // ``` + // + // Since the `
` tags don't have `key` attributes the Stencil vdom will + // re-use the same div element between re-renders, and will just swap out + // the children (the text nodes in this case). If our key insertion + // transformer put unique keys onto each tag then this wouldn't happen any + // longer. if (ts.isMethodDeclaration(node) && node.name.getText() === 'render' && numReturnStatements(node) === 1) { return ts.visitEachChild(node, jsxElementVisitor, transformCtx); } else { @@ -91,16 +113,15 @@ export const performAutomaticKeyInsertion = (transformCtx: ts.TransformationCont * @returns the result of handling the node */ function jsxElementVisitor(node: ts.Node): ts.VisitResult { - if (ts.isJsxExpression(node)) { - // we don't have the static analysis chops to dive into a JSX expression - // (arbitrary JS code delimited by curly braces in JSX) in order to - // determine whether it's safe to add keys to JSX nodes within it, so we - // bail here instead. + if (ts.isCallExpression(node)) { + // if there are any JSX nodes which are children of the call expression + // (i.e. arguments) we don't want to transform them since we can't know + // _a priori_ what could be done with them at runtime return node; } else if (ts.isConditionalExpression(node)) { // we're going to encounter the same problem here that we encounter with - // multiple return statements, so we don't try to transform the arms of - // the conditional. + // multiple return statements, so we just return the node and don't recur into + // its children return node; } else if (isJSXElWithAttrs(node)) { return addKeyAttr(node);