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 new file mode 100644 index 00000000000..01384222f78 --- /dev/null +++ b/src/compiler/transformers/automatic-key-insertion/automatic-key-insertion.spec.ts @@ -0,0 +1,315 @@ +import { transpileModule } from '../test/transpile'; +import { formatCode } from '../test/utils'; +import * as keyInsertionUtils from './utils'; + +function transpile(code: string) { + return transpileModule(code, null, null, []); +} + +describe('automatic key insertion', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should add a key to one JSX opener', async () => { + jest.spyOn(keyInsertionUtils, 'deriveJSXKey').mockReturnValue('test-key'); + const t = transpile(` + @Component({tag: 'cmp-a'}) + export class CmpA { + render() { + return
test
+ } + }`); + + expect(await formatCode(t.outputText)).toBe( + `export class CmpA { + render() { + return h('div', { key: 'test-key' }, 'test'); + } + static get is() { + return 'cmp-a'; + } +} +`, + ); + }); + + it('should add a key to nested JSX', async () => { + jest.spyOn(keyInsertionUtils, 'deriveJSXKey').mockReturnValueOnce('key1').mockReturnValueOnce('key2'); + const t = transpile(` + @Component({tag: 'cmp-a'}) + export class CmpA { + render() { + return
test
+ } + }`); + + expect(await formatCode(t.outputText)).toBe( + `export class CmpA { + render() { + return h('div', { key: 'key1' }, 'test', h('img', { key: 'key2', src: 'image.png' })); + } + static get is() { + return 'cmp-a'; + } +} +`, + ); + }); + + it('should add a key to one JSX opener w/ existing attr', async () => { + jest.spyOn(keyInsertionUtils, 'deriveJSXKey').mockReturnValue('test-key'); + const t = transpile(` + @Component({tag: 'cmp-a'}) + export class CmpA { + render() { + return
test
+ } + }`); + + expect(await formatCode(t.outputText)).toBe( + `export class CmpA { + render() { + return h('div', { key: 'test-key', class: 'foo' }, 'test'); + } + static get is() { + return 'cmp-a'; + } +} +`, + ); + }); + + it('should add a key to a self-closing JSX element', async () => { + jest.spyOn(keyInsertionUtils, 'deriveJSXKey').mockReturnValue('img-key'); + const t = transpile(` + @Component({tag: 'cmp-a'}) + export class CmpA { + render() { + return + } + }`); + + expect(await formatCode(t.outputText)).toBe( + `export class CmpA { + render() { + return h('img', { key: 'img-key' }); + } + static get is() { + return 'cmp-a'; + } +} +`, + ); + }); + + it('should add a key to a self-closing JSX element w/ existing attr', async () => { + jest.spyOn(keyInsertionUtils, 'deriveJSXKey').mockReturnValue('img-key'); + const t = transpile(` + @Component({tag: 'cmp-a'}) + export class CmpA { + render() { + return + } + }`); + + expect(await formatCode(t.outputText)).toBe( + `export class CmpA { + render() { + return h('img', { key: 'img-key', src: 'my-img.png' }); + } + static get is() { + return 'cmp-a'; + } +} +`, + ); + }); + + it('should add unique keys to multiple JSX elements', async () => { + jest.spyOn(keyInsertionUtils, 'deriveJSXKey').mockReturnValueOnce('first-key').mockReturnValueOnce('second-key'); + const t = transpile(` + @Component({tag: 'cmp-a'}) + export class CmpA { + render() { + return
+ } + }`); + + expect(await formatCode(t.outputText)).toBe( + `export class CmpA { + render() { + return h('div', { key: 'first-key' }, h('img', { key: 'second-key', src: 'my-img.png' })); + } + static get is() { + return 'cmp-a'; + } +} +`, + ); + }); + + it('should respect an existing key', async () => { + jest.spyOn(keyInsertionUtils, 'deriveJSXKey').mockReturnValue('never-key'); + const t = transpile(` + @Component({tag: 'cmp-a'}) + export class CmpA { + render() { + return
hey
+ } + }`); + + expect(await formatCode(t.outputText)).toBe( + `export class CmpA { + render() { + return h('div', { key: 'my-key' }, 'hey'); + } + static get is() { + return 'cmp-a'; + } +} +`, + ); + }); + + it('should respect an existing key in a loop', async () => { + jest.spyOn(keyInsertionUtils, 'deriveJSXKey').mockReturnValueOnce('once-key'); + const t = transpile(` + @Component({tag: 'cmp-a'}) + export class CmpA { + render() { + return ( +
+ {this.todos.map((todo) => ( +
{ todo }
+ ))} +
+ ) + } + }`); + expect(await formatCode(t.outputText)).toBe( + `export class CmpA { + render() { + return h( + 'div', + { key: 'once-key' }, + this.todos.map((todo) => h('div', { key: todo }, todo)), + ); + } + static get is() { + return 'cmp-a'; + } +} +`, + ); + }); + + it('should not add a static key to dynamic elements', async () => { + jest.spyOn(keyInsertionUtils, 'deriveJSXKey').mockReturnValueOnce('once-key'); + const t = transpile(` + @Component({tag: 'cmp-a'}) + export class CmpA { + render() { + return ( +
+ {this.todos.map((todo) => ( +
{ todo }
+ ))} +
+ ) + } + }`); + expect(await formatCode(t.outputText)).toBe( + `export class CmpA { + render() { + return h( + 'div', + { key: 'once-key' }, + this.todos.map((todo) => h('div', null, todo)), + ); + } + static get is() { + return 'cmp-a'; + } +} +`, + ); + }); + + it('should not transform JSX inside of a ternary', async () => { + jest.spyOn(keyInsertionUtils, 'deriveJSXKey').mockReturnValue('shouldnt-see-key'); + const t = transpile(` + @Component({tag: 'cmp-a'}) + export class CmpA { + yes = false; + render() { + return this.yes ? yes : no + } + }`); + expect(await formatCode(t.outputText)).toBe( + `export class CmpA { + constructor() { + this.yes = false; + } + render() { + return this.yes ? h('span', null, 'yes') : h('span', null, 'no'); + } + 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(` + @Component({tag: 'cmp-a'}) + export class CmpA { + booleo = false; + render() { + if (this.booleo) { + return
early!
; + } + return
late!
; + } + }`); + expect(await formatCode(t.outputText)).toBe( + `export class CmpA { + constructor() { + this.booleo = false; + } + render() { + if (this.booleo) { + return h('div', null, 'early!'); + } + return h('div', null, 'late!'); + } + static get is() { + return 'cmp-a'; + } +} +`, + ); + }); + + it('should not edit a non-stencil class', async () => { + jest.spyOn(keyInsertionUtils, 'deriveJSXKey').mockReturnValue("shouldn't see this!"); + const t = transpile(` + export class CmpA { + render() { + return
hey
+ } + }`); + + expect(await formatCode(t.outputText)).toBe( + `export class CmpA { + render() { + return h('div', null, 'hey'); + } +} +`, + ); + }); +}); diff --git a/src/compiler/transformers/automatic-key-insertion/index.ts b/src/compiler/transformers/automatic-key-insertion/index.ts new file mode 100644 index 00000000000..8fab80f0505 --- /dev/null +++ b/src/compiler/transformers/automatic-key-insertion/index.ts @@ -0,0 +1,232 @@ +import ts from 'typescript'; + +import { getComponentTagName, isStaticGetter } from '../transform-utils'; +import { deriveJSXKey } from './utils'; + +/** + * A transformer factory to create a transformer which will add `key` + * properties to all of the JSX nodes contained inside of a Stencil component's + * `render` function. + * + * This can be thought of as transforming the following: + * + * ```tsx + * class MyComponent { + * render() { + *
hey!
+ * } + * } + * ``` + * + * to this: + * + * ```tsx + * class MyComponent { + * render() { + *
hey!
+ * } + * } + * ``` + * + * The inserted keys are generated by {@link deriveJSXKey}. + * + * **Note**: this transformer must be run _after_ the + * `convertDecoratorsToStatic` transformer, since it depends on static getters + * created by that transformer to determine when to transform a class node. + * + * @param transformCtx a transformation context + * @returns a typescript transformer for inserting keys into JSX nodes + */ +export const performAutomaticKeyInsertion = (transformCtx: ts.TransformationContext): ts.Transformer => { + /** + * This is our outer-most visitor function which serves to locate a class + * declaration which is also a Stencil component, 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 + * @returns the result of handling the node + */ + function findClassDeclVisitor(node: ts.Node): ts.VisitResult { + if (ts.isClassDeclaration(node)) { + const tagName = getComponentTagName(node.members.filter(isStaticGetter)); + if (tagName != null) { + // we've got a class node with an `is` property, which tells us that + // the class we're dealing with is a Stencil component which has + // already been through the `convertDecoratorsToStatic` transformer. + return ts.visitEachChild(node, findRenderMethodVisitor, transformCtx); + } + } + // we either didn't find a class node, or we found a class node without a + // component tag name, so this is not a stencil component! + 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 + * @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: + // + // 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 + if (ts.isMethodDeclaration(node) && node.name.getText() === 'render' && numReturnStatements(node) === 1) { + 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 + * @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. + 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. + return node; + } else if (isJSXElWithAttrs(node)) { + return addKeyAttr(node); + } else { + return ts.visitEachChild(node, jsxElementVisitor, transformCtx); + } + } + + return (tsSourceFile) => { + return ts.visitEachChild(tsSourceFile, findClassDeclVisitor, transformCtx); + }; +}; + +/** + * Count the number of return statements in a {@link ts.MethodDeclaration} + * + * @param method the node within which we're going to count `return` statements + * @returns the number of return statements found + */ +function numReturnStatements(method: ts.MethodDeclaration): number { + let count = 0; + + function walker(node: ts.Node) { + for (const child of node.getChildren()) { + if (ts.isReturnStatement(child)) { + count++; + } else { + walker(child); + } + } + } + + walker(method); + + return count; +} + +/** + * Type guard to see if a TypeScript syntax node is one of the node types which + * corresponds to a JSX element that can have attributes on it. This is either + * an opening node, like `
`, or a 'self-closing' node like + * ``. + * + * @param node a typescript syntax tree node + * @returns whether or not the node is JSX node which could have attributes + */ +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. + * This will respect any attributes already set on the node, including a + * pre-existing, user-defined `key` attribute. + * + * @param jsxElement a typescript JSX syntax tree node + * @returns an updated JSX element, with a key added. + */ +function addKeyAttr( + jsxElement: ts.JsxOpeningElement | ts.JsxSelfClosingElement, +): ts.JsxOpeningElement | ts.JsxSelfClosingElement { + if (jsxElement.attributes.properties.some(isKeyAttr)) { + // this node already has a key! let's get out of here + return jsxElement; + } + + const updatedAttributes = ts.factory.createJsxAttributes([ + ts.factory.createJsxAttribute( + ts.factory.createIdentifier('key'), + ts.factory.createStringLiteral(deriveJSXKey(jsxElement)), + ), + ...jsxElement.attributes.properties, + ]); + + if (ts.isJsxOpeningElement(jsxElement)) { + return ts.factory.updateJsxOpeningElement( + jsxElement, + jsxElement.tagName, + jsxElement.typeArguments, + updatedAttributes, + ); + } else { + return ts.factory.updateJsxSelfClosingElement( + jsxElement, + jsxElement.tagName, + jsxElement.typeArguments, + updatedAttributes, + ); + } +} + +/** + * Check whether or not a JSX attribute node (well, technically a + * {@link ts.JsxAttributeLike} node) has the name `"key"` or not + * + * @param attr the JSX attribute node to check + * @returns whether or not this node has the name 'key' + */ +function isKeyAttr(attr: ts.JsxAttributeLike): boolean { + return !!attr.name && attrNameToString(attr) === 'key'; +} + +/** + * Given a JSX attribute get its name as a string + * + * @param attr the attribute of interest + * @returns the attribute's name, formatted as a string + */ +function attrNameToString(attr: ts.JsxAttributeLike): string { + switch (attr.name?.kind) { + case ts.SyntaxKind.Identifier: + case ts.SyntaxKind.PrivateIdentifier: + case ts.SyntaxKind.StringLiteral: + case ts.SyntaxKind.NumericLiteral: + return attr.name.text; + case ts.SyntaxKind.JsxNamespacedName: + // this is a JSX attribute name like `foo:bar` + // see https://facebook.github.io/jsx/#prod-JSXNamespacedName + return attr.name.getText(); + case ts.SyntaxKind.ComputedPropertyName: + const expression = attr.name.expression; + if (ts.isStringLiteral(expression) || ts.isNumericLiteral(expression)) { + return expression.text; + } + return ''; + default: + return ''; + } +} diff --git a/src/compiler/transformers/automatic-key-insertion/utils.ts b/src/compiler/transformers/automatic-key-insertion/utils.ts new file mode 100644 index 00000000000..8d854dc77aa --- /dev/null +++ b/src/compiler/transformers/automatic-key-insertion/utils.ts @@ -0,0 +1,36 @@ +import { createHash } from 'node:crypto'; + +import ts from 'typescript'; + +/** + * 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 + */ +export 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; +} diff --git a/src/compiler/transformers/test/transpile.ts b/src/compiler/transformers/test/transpile.ts index 97adedaad36..48bd4b7419d 100644 --- a/src/compiler/transformers/test/transpile.ts +++ b/src/compiler/transformers/test/transpile.ts @@ -2,6 +2,7 @@ import type * as d from '@stencil/core/declarations'; import { mockBuildCtx, mockCompilerCtx, mockValidatedConfig } from '@stencil/core/testing'; import ts from 'typescript'; +import { performAutomaticKeyInsertion } from '../automatic-key-insertion'; import { convertDecoratorsToStatic } from '../decorators-to-static/convert-decorators'; import { updateModule } from '../static-to-meta/parse-static'; import { convertStaticToMeta } from '../static-to-meta/visitor'; @@ -109,7 +110,11 @@ export function transpileModule( }; tsProgram.emit(undefined, undefined, undefined, undefined, { - before: [convertDecoratorsToStatic(config, buildCtx.diagnostics, tsTypeChecker, tsProgram), ...beforeTransformers], + before: [ + convertDecoratorsToStatic(config, buildCtx.diagnostics, tsTypeChecker, tsProgram), + performAutomaticKeyInsertion, + ...beforeTransformers, + ], after: [ convertStaticToMeta(config, compilerCtx, buildCtx, tsTypeChecker, null, transformOpts), ...afterTransformers, diff --git a/src/compiler/transpile/run-program.ts b/src/compiler/transpile/run-program.ts index 70b4da1d8fc..129650a565d 100644 --- a/src/compiler/transpile/run-program.ts +++ b/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: [], }; diff --git a/src/compiler/transpile/transpile-module.ts b/src/compiler/transpile/transpile-module.ts index b5fb480e108..26d173cda50 100644 --- a/src/compiler/transpile/transpile-module.ts +++ b/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)], diff --git a/src/runtime/test/render-vdom.spec.tsx b/src/runtime/test/render-vdom.spec.tsx index c340d0b0a85..8a6c1f68bac 100644 --- a/src/runtime/test/render-vdom.spec.tsx +++ b/src/runtime/test/render-vdom.spec.tsx @@ -15,11 +15,11 @@ describe('render-vdom', () => { const { build } = await newSpecPage({ components: [CmpA], strictBuild: true }); expect(build).toMatchObject({ - vdomAttribute: false, + vdomAttribute: true, vdomXlink: false, vdomClass: false, vdomStyle: false, - vdomKey: false, + vdomKey: true, vdomRef: false, vdomListener: false, vdomFunctional: false, @@ -38,11 +38,11 @@ describe('render-vdom', () => { const { build } = await newSpecPage({ components: [CmpA], strictBuild: true }); expect(build).toMatchObject({ - vdomAttribute: false, + vdomAttribute: true, vdomXlink: false, vdomClass: false, vdomStyle: false, - vdomKey: false, + vdomKey: true, vdomRef: false, vdomListener: false, vdomFunctional: false, @@ -61,11 +61,11 @@ describe('render-vdom', () => { const { build } = await newSpecPage({ components: [CmpA], strictBuild: true }); expect(build).toMatchObject({ - vdomAttribute: false, + vdomAttribute: true, vdomXlink: false, vdomClass: false, vdomStyle: false, - vdomKey: false, + vdomKey: true, vdomRef: false, vdomListener: false, vdomFunctional: false, @@ -84,11 +84,11 @@ describe('render-vdom', () => { const { build } = await newSpecPage({ components: [CmpA], strictBuild: true }); expect(build).toMatchObject({ - vdomAttribute: false, + vdomAttribute: true, vdomXlink: false, vdomClass: false, vdomStyle: false, - vdomKey: false, + vdomKey: true, vdomRef: false, vdomListener: false, vdomFunctional: false, @@ -110,7 +110,7 @@ describe('render-vdom', () => { vdomXlink: false, vdomClass: true, vdomStyle: false, - vdomKey: false, + vdomKey: true, vdomRef: false, vdomListener: false, vdomFunctional: false, @@ -132,7 +132,7 @@ describe('render-vdom', () => { vdomXlink: false, vdomClass: false, vdomStyle: true, - vdomKey: false, + vdomKey: true, vdomRef: false, vdomListener: false, vdomFunctional: false, @@ -182,7 +182,7 @@ describe('render-vdom', () => { vdomXlink: false, vdomClass: false, vdomStyle: false, - vdomKey: false, + vdomKey: true, vdomRef: true, vdomListener: false, vdomFunctional: false, @@ -210,7 +210,7 @@ describe('render-vdom', () => { vdomXlink: false, vdomClass: false, vdomStyle: false, - vdomKey: false, + vdomKey: true, vdomRef: false, vdomListener: true, vdomFunctional: false, @@ -238,7 +238,7 @@ describe('render-vdom', () => { vdomXlink: false, vdomClass: false, vdomStyle: false, - vdomKey: false, + vdomKey: true, vdomRef: false, vdomListener: true, vdomFunctional: false, @@ -337,7 +337,7 @@ describe('render-vdom', () => { vdomXlink: false, vdomClass: false, vdomStyle: false, - vdomKey: false, + vdomKey: true, vdomRef: false, vdomListener: false, vdomFunctional: false, @@ -1073,8 +1073,10 @@ describe('render-vdom', () => { @Component({ tag: 'cmp-a' }) class CmpA { counter = 0; - setRef = () => { - this.counter++; + setRef = (el: HTMLDivElement | null) => { + if (el !== null) { + this.counter++; + } }; @Prop() state = true; diff --git a/test/karma/test-app/components.d.ts b/test/karma/test-app/components.d.ts index 32369fd22f1..6d3a02a9e6a 100644 --- a/test/karma/test-app/components.d.ts +++ b/test/karma/test-app/components.d.ts @@ -252,6 +252,9 @@ export namespace Components { } interface ScopedBasicRoot { } + interface ScopedConditional { + "renderHello": boolean; + } interface ScopedSlotAppendAndPrepend { } interface ScopedSlotChildInsertAdjacent { @@ -1076,6 +1079,12 @@ declare global { prototype: HTMLScopedBasicRootElement; new (): HTMLScopedBasicRootElement; }; + interface HTMLScopedConditionalElement extends Components.ScopedConditional, HTMLStencilElement { + } + var HTMLScopedConditionalElement: { + prototype: HTMLScopedConditionalElement; + new (): HTMLScopedConditionalElement; + }; interface HTMLScopedSlotAppendAndPrependElement extends Components.ScopedSlotAppendAndPrepend, HTMLStencilElement { } var HTMLScopedSlotAppendAndPrependElement: { @@ -1547,6 +1556,7 @@ declare global { "sass-cmp": HTMLSassCmpElement; "scoped-basic": HTMLScopedBasicElement; "scoped-basic-root": HTMLScopedBasicRootElement; + "scoped-conditional": HTMLScopedConditionalElement; "scoped-slot-append-and-prepend": HTMLScopedSlotAppendAndPrependElement; "scoped-slot-child-insert-adjacent": HTMLScopedSlotChildInsertAdjacentElement; "shadow-dom-array": HTMLShadowDomArrayElement; @@ -1861,6 +1871,9 @@ declare namespace LocalJSX { } interface ScopedBasicRoot { } + interface ScopedConditional { + "renderHello"?: boolean; + } interface ScopedSlotAppendAndPrepend { } interface ScopedSlotChildInsertAdjacent { @@ -2100,6 +2113,7 @@ declare namespace LocalJSX { "sass-cmp": SassCmp; "scoped-basic": ScopedBasic; "scoped-basic-root": ScopedBasicRoot; + "scoped-conditional": ScopedConditional; "scoped-slot-append-and-prepend": ScopedSlotAppendAndPrepend; "scoped-slot-child-insert-adjacent": ScopedSlotChildInsertAdjacent; "shadow-dom-array": ShadowDomArray; @@ -2261,6 +2275,7 @@ declare module "@stencil/core" { "sass-cmp": LocalJSX.SassCmp & JSXBase.HTMLAttributes; "scoped-basic": LocalJSX.ScopedBasic & JSXBase.HTMLAttributes; "scoped-basic-root": LocalJSX.ScopedBasicRoot & JSXBase.HTMLAttributes; + "scoped-conditional": LocalJSX.ScopedConditional & JSXBase.HTMLAttributes; "scoped-slot-append-and-prepend": LocalJSX.ScopedSlotAppendAndPrepend & JSXBase.HTMLAttributes; "scoped-slot-child-insert-adjacent": LocalJSX.ScopedSlotChildInsertAdjacent & JSXBase.HTMLAttributes; "shadow-dom-array": LocalJSX.ShadowDomArray & JSXBase.HTMLAttributes; diff --git a/test/karma/test-app/scoped-conditional/cmp.tsx b/test/karma/test-app/scoped-conditional/cmp.tsx new file mode 100644 index 00000000000..f0b40580351 --- /dev/null +++ b/test/karma/test-app/scoped-conditional/cmp.tsx @@ -0,0 +1,31 @@ +import { Component, h, Prop } from '@stencil/core'; + +@Component({ + tag: 'scoped-conditional', + scoped: true, +}) +export class ScopedConditional { + @Prop() renderHello: boolean = false; + + render() { + return ( +
+ {/* prior to fixing the bug */} + {/* - if you remove the conditional below, it works */} + {/* - if you remove the
around `.tag`, it works */} + {/* - if you add additional elements between the conditional and the second
, 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
, */} + {/* we get a separate error where the slot doesn't get put in the correct place */} + {this.renderHello &&
} + {/* This div below must be there too */} +
+ before slot-> + + <-after slot +
+
+ ); + } +} diff --git a/test/karma/test-app/scoped-conditional/index.html b/test/karma/test-app/scoped-conditional/index.html new file mode 100644 index 00000000000..4f823a9f143 --- /dev/null +++ b/test/karma/test-app/scoped-conditional/index.html @@ -0,0 +1,16 @@ + + + + + + + + + +
This div will be slotted in
+
diff --git a/test/karma/test-app/scoped-conditional/karma.spec.ts b/test/karma/test-app/scoped-conditional/karma.spec.ts new file mode 100644 index 00000000000..db0b621bf74 --- /dev/null +++ b/test/karma/test-app/scoped-conditional/karma.spec.ts @@ -0,0 +1,61 @@ +import { setupDomTests, waitForChanges } from '../util'; + +describe('scoped-conditional', () => { + const { setupDom, tearDownDom } = setupDomTests(document); + let app: HTMLElement | undefined; + + beforeEach(async () => { + app = await setupDom('/scoped-conditional/index.html'); + }); + + afterEach(tearDownDom); + + it('renders the initial slotted content', () => { + const host = app.querySelector('scoped-conditional'); + const outerDiv = host.querySelector('div'); + + expect(outerDiv.textContent).toBe( + `before slot-> + This div will be slotted in +<-after slot`, + ); + }); + + it('renders the slotted content after toggling the message', async () => { + // toggle the 'Hello' message, which should insert a new
into the DOM & _not_ remove the slotted content + const toggleButton = app.querySelector('#toggleHello'); + toggleButton.click(); + await waitForChanges(); + + const host = app.querySelector('scoped-conditional'); + const outerDivChildren = host.querySelector('div').childNodes; + + expect(outerDivChildren.length).toBe(2); + + expect(outerDivChildren[0].textContent).toBe('Hello'); + expect(outerDivChildren[1].textContent).toBe( + `before slot-> + This div will be slotted in +<-after slot`, + ); + }); + + it('renders the slotted content after toggling the twice message', async () => { + // toggle the 'Hello' message twice, which should insert a new
into the DOM, then remove it. + // as a result of the toggle, we should _not_ remove the slotted content + const toggleButton = app.querySelector('#toggleHello'); + toggleButton.click(); + await waitForChanges(); + toggleButton.click(); + await waitForChanges(); + + const host = app.querySelector('scoped-conditional'); + const outerDiv = host.querySelector('div'); + + expect(outerDiv.textContent).toBe( + `before slot-> + This div will be slotted in +<-after slot`, + ); + }); +}); diff --git a/test/karma/test-app/svg-attr/karma.spec.ts b/test/karma/test-app/svg-attr/karma.spec.ts index c2055699096..dcd1d655d18 100644 --- a/test/karma/test-app/svg-attr/karma.spec.ts +++ b/test/karma/test-app/svg-attr/karma.spec.ts @@ -10,16 +10,18 @@ describe('svg attr', () => { afterEach(tearDownDom); it('adds and removes attribute', async () => { - const rect = app.querySelector('rect'); + let rect = app.querySelector('rect'); expect(rect.getAttribute('transform')).toBe(null); const button = app.querySelector('button'); button.click(); await waitForChanges(); + rect = app.querySelector('rect'); expect(rect.getAttribute('transform')).toBe('rotate(45 27 27)'); button.click(); await waitForChanges(); + rect = app.querySelector('rect'); expect(rect.getAttribute('transform')).toBe(null); }); });