From 8413ed3ac73dff14d27674bd5f2d608a8fc7f073 Mon Sep 17 00:00:00 2001 From: Andrew Jakubowicz Date: Mon, 6 Dec 2021 12:14:04 -0800 Subject: [PATCH] Address ts-transformers feedback and allow shorthand properties. In this change made the transformer far more flexible. It no longer forces string literal arguments, and instead utilizes template strings to handle arbitrary expressions or identifiers. --- .../decorators/query-assigned-elements.ts | 95 +++++++++----- .../src/tests/idiomatic-decorators-test.ts | 119 +++++++++++++----- 2 files changed, 156 insertions(+), 58 deletions(-) diff --git a/packages/ts-transformers/src/internal/decorators/query-assigned-elements.ts b/packages/ts-transformers/src/internal/decorators/query-assigned-elements.ts index 8ed236dd1e..d3a861bb30 100644 --- a/packages/ts-transformers/src/internal/decorators/query-assigned-elements.ts +++ b/packages/ts-transformers/src/internal/decorators/query-assigned-elements.ts @@ -56,7 +56,13 @@ export class QueryAssignedElementsVisitor implements MemberDecoratorVisitor { `object literal. Instead received: '${arg0.getText()}'` ); } - if (arg0 && arg0.properties.some((p) => !ts.isPropertyAssignment(p))) { + if ( + arg0 && + arg0.properties.some( + (p) => + !(ts.isPropertyAssignment(p) || ts.isShorthandPropertyAssignment(p)) + ) + ) { throw new Error( `queryAssignedElements object literal argument can only include ` + `property assignment. For example: '{ slot: "example" }' is ` + @@ -66,44 +72,44 @@ export class QueryAssignedElementsVisitor implements MemberDecoratorVisitor { const {slot, selector} = this._retrieveSlotAndSelector(arg0); litClassContext.litFileContext.replaceAndMoveComments( property, - this._createQueryAssignedElementsGetter( + this._createQueryAssignedElementsGetter({ name, slot, selector, - this._filterAssignedElementsOptions(arg0) - ) + assignedElsOptions: this._filterAssignedElementsOptions(arg0), + }) ); } + /** + * @param opts object literal node passed into the queryAssignedElements decorator + * @returns expression nodes for the slot and selector. + */ private _retrieveSlotAndSelector(opts?: ts.ObjectLiteralExpression): { - slot: string; - selector: string; + slot?: ts.Expression; + selector?: ts.Expression; } { if (!opts) { - return {slot: '', selector: ''}; + return {}; } - const findStringLiteralFor = (key: string): string => { + const findExpressionFor = (key: string): ts.Expression | undefined => { const propAssignment = opts.properties.find( (p) => p.name && ts.isIdentifier(p.name) && p.name.text === key ); if (!propAssignment) { - return ''; + return; } - if ( - propAssignment && - ts.isPropertyAssignment(propAssignment) && - ts.isStringLiteral(propAssignment.initializer) - ) { - return propAssignment.initializer.text; + if (ts.isPropertyAssignment(propAssignment)) { + return propAssignment.initializer; } - throw new Error( - `queryAssignedElements object literal property '${key}' must be a ` + - `string literal.` - ); + if (ts.isShorthandPropertyAssignment(propAssignment)) { + return propAssignment.name; + } + return; }; return { - slot: findStringLiteralFor('slot'), - selector: findStringLiteralFor('selector'), + slot: findExpressionFor('slot'), + selector: findExpressionFor('selector'), }; } @@ -150,15 +156,22 @@ export class QueryAssignedElementsVisitor implements MemberDecoratorVisitor { ); } - private _createQueryAssignedElementsGetter( - name: string, - slot: string, - selector: string, - assignedElsOptions?: ts.ObjectLiteralExpression - ) { + private _createQueryAssignedElementsGetter({ + name, + slot, + selector, + assignedElsOptions, + }: { + name: string; + slot?: ts.Expression; + selector?: ts.Expression; + assignedElsOptions?: ts.ObjectLiteralExpression; + }) { const factory = this._factory; - const slotSelector = `slot${slot ? `[name=${slot}]` : ':not([name])'}`; + const slotSelector = slot + ? this.createNamedSlotSelector(slot) + : this.createDefaultSlotSelector(); const assignedElementsOptions = assignedElsOptions ? [assignedElsOptions] @@ -178,7 +191,7 @@ export class QueryAssignedElementsVisitor implements MemberDecoratorVisitor { ), undefined, undefined, - [factory.createStringLiteral(slotSelector)] + [slotSelector] ), factory.createToken(ts.SyntaxKind.QuestionDotToken), factory.createIdentifier('assignedElements') @@ -222,7 +235,7 @@ export class QueryAssignedElementsVisitor implements MemberDecoratorVisitor { factory.createIdentifier('matches') ), undefined, - [factory.createStringLiteral(selector)] + [selector] ) ), ] @@ -251,4 +264,26 @@ export class QueryAssignedElementsVisitor implements MemberDecoratorVisitor { getterBody ); } + + /** + * @param slot Expression that evaluates to the slot name. + * @returns Template string node representing `slot[name=${slot}]` + */ + private createNamedSlotSelector(slot: ts.Expression): ts.TemplateExpression { + const factory = this._factory; + return factory.createTemplateExpression( + factory.createTemplateHead('slot[name=', 'slot[name='), + [factory.createTemplateSpan(slot, factory.createTemplateTail(']', ']'))] + ); + } + + /** + * @returns Template string node representing `slot:not([name])` + */ + private createDefaultSlotSelector() { + return this._factory.createNoSubstitutionTemplateLiteral( + 'slot:not([name])', + 'slot:not([name])' + ); + } } diff --git a/packages/ts-transformers/src/tests/idiomatic-decorators-test.ts b/packages/ts-transformers/src/tests/idiomatic-decorators-test.ts index c597b992d5..c8b1886791 100644 --- a/packages/ts-transformers/src/tests/idiomatic-decorators-test.ts +++ b/packages/ts-transformers/src/tests/idiomatic-decorators-test.ts @@ -588,7 +588,7 @@ const tests = (test: uvu.Test, options: ts.CompilerOptions) => { // listItems comment get listItems() { return this.renderRoot - ?.querySelector('slot:not([name])') + ?.querySelector(\`slot:not([name])\`) ?.assignedElements() ?? []; } @@ -617,7 +617,7 @@ const tests = (test: uvu.Test, options: ts.CompilerOptions) => { // listItems comment get listItems() { return this.renderRoot - ?.querySelector('slot[name=list]') + ?.querySelector(\`slot[name=\${"list"}]\`) ?.assignedElements() ?? []; } } @@ -644,7 +644,7 @@ const tests = (test: uvu.Test, options: ts.CompilerOptions) => { // listItems comment get listItems() { return this.renderRoot - ?.querySelector('slot[name=list]') + ?.querySelector(\`slot[name=\${"list"}]\`) ?.assignedElements({flatten: true}) ?? []; } } @@ -671,7 +671,7 @@ const tests = (test: uvu.Test, options: ts.CompilerOptions) => { // listItems comment get listItems() { return this.renderRoot - ?.querySelector('slot[name=list]') + ?.querySelector(\`slot[name=\${"list"}]\`) ?.assignedElements({ flatten: false }) ?.filter((node) => node.matches('.item') ) ?? []; @@ -706,9 +706,9 @@ const tests = (test: uvu.Test, options: ts.CompilerOptions) => { // listItems comment get listItems() { return this.renderRoot - ?.querySelector('slot[name=list]') + ?.querySelector(\`slot[name=\${"list"}]\`) ?.assignedElements({ flatten: isFlatten }) - ?.filter((node) => node.matches('.item') + ?.filter((node) => node.matches(".item") ) ?? []; } } @@ -716,77 +716,140 @@ const tests = (test: uvu.Test, options: ts.CompilerOptions) => { checkTransform(input, expected, options); }); - test('@queryAssignedElements (fails if not object literal)', () => { + test('@queryAssignedElements (with slot and selector identifiers)', () => { const input = ` import {LitElement} from 'lit'; import {queryAssignedElements} from 'lit/decorators.js'; - const someIdentifier = {slot: 'list'}; + const slot = 'list'; + const selector = '.item'; class MyElement extends LitElement { // listItems comment - @queryAssignedElements(someIdentifier) + @queryAssignedElements({slot: slot, selector: selector}) listItems: HTMLElement[]; } `; - assert.throws( - () => checkTransform(input, '', options), - /expected to be an inlined object literal/ - ); + + const expected = ` + import {LitElement} from 'lit'; + + const slot = 'list'; + const selector = '.item'; + + class MyElement extends LitElement { + // listItems comment + get listItems() { + return this.renderRoot + ?.querySelector(\`slot[name=\${slot}]\`) + ?.assignedElements() + ?.filter((node) => node.matches(selector) + ) ?? []; + } + } + `; + checkTransform(input, expected, options); }); - test('@queryAssignedElements (fails if not property assignment - spread)', () => { + test('@queryAssignedElements (shorthand properties)', () => { const input = ` import {LitElement} from 'lit'; import {queryAssignedElements} from 'lit/decorators.js'; + const slot = 'list'; + const selector = '.item'; + const flatten: boolean = false; + class MyElement extends LitElement { // listItems comment - @queryAssignedElements({slot: 'list', ...{}}) + @queryAssignedElements({slot, selector, flatten}) listItems: HTMLElement[]; } `; - assert.throws( - () => checkTransform(input, '', options), - /argument can only include property assignment/ - ); + + const expected = ` + import {LitElement} from 'lit'; + + const slot = 'list'; + const selector = '.item'; + const flatten = false; + + class MyElement extends LitElement { + // listItems comment + get listItems() { + return this.renderRoot + ?.querySelector(\`slot[name=\${slot}]\`) + ?.assignedElements({ flatten }) + ?.filter((node) => node.matches(selector) + ) ?? []; + } + } + `; + checkTransform(input, expected, options); }); - test('@queryAssignedElements (fails if not property assignment - shorthand)', () => { + test('@queryAssignedElements (arbitrary inline expressions)', () => { const input = ` import {LitElement} from 'lit'; import {queryAssignedElements} from 'lit/decorators.js'; - const slot = "shorthandSyntaxInvalid"; + class MyElement extends LitElement { + // listItems comment + @queryAssignedElements({slot: "li" + "st", selector: "." + "item", flatten: true || false}) + listItems: HTMLElement[]; + } + `; + + const expected = ` + import {LitElement} from 'lit'; class MyElement extends LitElement { // listItems comment - @queryAssignedElements({slot}) + get listItems() { + return this.renderRoot + ?.querySelector(\`slot[name=\${"li" + "st"}]\`) + ?.assignedElements({ flatten: true || false }) + ?.filter((node) => node.matches("." + "item") + ) ?? []; + } + } + `; + checkTransform(input, expected, options); + }); + + test('@queryAssignedElements (fails if not object literal)', () => { + const input = ` + import {LitElement} from 'lit'; + import {queryAssignedElements} from 'lit/decorators.js'; + + const someIdentifier = {slot: 'list'}; + + class MyElement extends LitElement { + // listItems comment + @queryAssignedElements(someIdentifier) listItems: HTMLElement[]; } `; assert.throws( () => checkTransform(input, '', options), - /argument can only include property assignment/ + /expected to be an inlined object literal/ ); }); - test('@queryAssignedElements (fail if slot or selector not literal)', () => { + test('@queryAssignedElements (fails if not property assignment - spread)', () => { const input = ` import {LitElement} from 'lit'; import {queryAssignedElements} from 'lit/decorators.js'; - const slot = 'list'; - class MyElement extends LitElement { // listItems comment - @queryAssignedElements({slot: slot}) + @queryAssignedElements({slot: 'list', ...{}}) listItems: HTMLElement[]; } `; assert.throws( () => checkTransform(input, '', options), - /property 'slot' must be a string literal/ + /argument can only include property assignment/ ); });