From a6bf833d2665fd3347cc9baa262a02f6983e7321 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Thu, 19 Dec 2024 20:09:30 -0300 Subject: [PATCH 01/11] feat(ssr): validate api decorator --- .../src/__tests__/compilation.spec.ts | 119 ++++++++++++++++++ .../ssr-compiler/src/compile-js/errors.ts | 15 ++- .../src/compile-js/generate-markup.ts | 4 +- .../@lwc/ssr-compiler/src/compile-js/index.ts | 38 +++++- .../@lwc/ssr-compiler/src/compile-js/types.ts | 4 +- 5 files changed, 169 insertions(+), 11 deletions(-) diff --git a/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts b/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts index 95e75a688b..09c606e7a9 100644 --- a/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts +++ b/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts @@ -2,6 +2,12 @@ import path from 'node:path'; import { describe, test, expect } from 'vitest'; import { compileComponentForSSR } from '../index'; +const compile = + (src: string, filename = 'test.js') => + () => { + return compileComponentForSSR(src, filename, {}); + }; + describe('component compilation', () => { test('implicit templates imports do not use full file paths', () => { const src = ` @@ -32,3 +38,116 @@ describe('component compilation', () => { expect(code).toContain('import tmpl from "./component.html"'); }); }); + +describe('component compilation errors', () => { + describe('api decorator', () => { + test('conflicting api properties with getter/setter', () => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Text extends LightningElement { + @api foo = 1; + + _internal = 1; + + @api + get foo() { + return "foo"; + } + set foo(val) { + this._internal = val; + } + } + `) + ).toThrow(`LWC1096: Duplicate @api property "foo".`); + }); + + test('conflicting api properties with method', () => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Text extends LightningElement { + @api foo = 1; + @api foo() { + return "foo"; + } + } + `) + ).toThrow(`LWC1096: Duplicate @api property "foo".`); + }); + + test('detecting @api on both getter and a setter should produce an error', () => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @api + get something() { + return this.s; + } + @api + set something(value) { + this.s = value; + } + } + `) + ).toThrow( + `LWC1112: @api get something and @api set something detected in class declaration. Only one of the two needs to be decorated with @api.` + ); + }); + + describe('disallowed props', () => { + test('class', () => { + expect( + compile(/* js */ ` + import { LightningElement, api } from 'lwc' + + export default class extends LightningElement { + @api class + } + `) + ).toThrow( + `LWC1110: Invalid property name "class". "class" is a reserved attribute.` + ); + }); + + test('is', () => { + expect( + compile(/* js */ ` + import { LightningElement, api } from 'lwc' + + export default class extends LightningElement { + @api is + } + `) + ).toThrow(`LWC1110: Invalid property name "is". "is" is a reserved attribute.`); + }); + + test('slot', () => { + expect( + compile(/* js */ ` + import { LightningElement, api } from 'lwc' + + export default class extends LightningElement { + @api slot + } + `) + ).toThrow(`LWC1110: Invalid property name "slot". "slot" is a reserved attribute.`); + }); + + test('style', () => { + expect( + compile(/* js */ ` + import { LightningElement, api } from 'lwc' + + export default class extends LightningElement { + @api style + } + `) + ).toThrow( + `LWC1110: Invalid property name "style". "style" is a reserved attribute.` + ); + }); + }); + }); +}); diff --git a/packages/@lwc/ssr-compiler/src/compile-js/errors.ts b/packages/@lwc/ssr-compiler/src/compile-js/errors.ts index 1a57ad5e4b..b5d3bb81a5 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/errors.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/errors.ts @@ -17,9 +17,20 @@ type ExtractArguments< : ExtractArguments // `N` already accounted for : Args; // No `N` found, nothing more to check +class CompilationError extends Error { + constructor( + message: string, + public code: number + ) { + super(message); + this.name = 'CompilationError'; + this.code = code; + } +} + export function generateError( error: T, ...args: ExtractArguments -): Error { - return new Error(generateErrorMessage(error, args)); +): CompilationError { + return new CompilationError(generateErrorMessage(error, args), error.code); } diff --git a/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts b/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts index 34012923be..fb29f97ba1 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts @@ -145,8 +145,8 @@ export function addGenerateMarkupFunction( program.body.push( ...bGenerateMarkup( defaultTagName, - b.arrayExpression(publicFields.map(b.literal)), - b.arrayExpression(privateFields.map(b.literal)), + b.arrayExpression([...publicFields.keys()].map(b.literal)), + b.arrayExpression([...privateFields].map(b.literal)), classIdentifier, connectWireAdapterCode ) diff --git a/packages/@lwc/ssr-compiler/src/compile-js/index.ts b/packages/@lwc/ssr-compiler/src/compile-js/index.ts index e83f8de2e4..389dca0e1c 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/index.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/index.ts @@ -101,16 +101,24 @@ const visitors: Visitors = { validateUniqueDecorator(decorators); const decoratedExpression = decorators?.[0]?.expression; if (is.identifier(decoratedExpression) && decoratedExpression.name === 'api') { - state.publicFields.push(node.key.name); + if (state.publicFields.has(node.key.name)) { + throw generateError(DecoratorErrors.DUPLICATE_API_PROPERTY, node.key.name); + } + + if (['class', 'is', 'slot', 'style'].includes(node.key.name)) { + throw generateError(DecoratorErrors.PROPERTY_NAME_IS_RESERVED, node.key.name); + } + + state.publicFields.set(node.key.name, node); } else if ( is.callExpression(decoratedExpression) && is.identifier(decoratedExpression.callee) && decoratedExpression.callee.name === 'wire' ) { catalogWireAdapters(path, state); - state.privateFields.push(node.key.name); + state.privateFields.add(node.key.name); } else { - state.privateFields.push(node.key.name); + state.privateFields.add(node.key.name); } if ( @@ -166,6 +174,26 @@ const visitors: Visitors = { } } + if (is.identifier(decoratedExpression, { name: 'api' })) { + const field = state.publicFields.get(node.key.name); + if (!field) { + state.publicFields.set(node.key.name, node); + } else { + if ( + (is.methodDefinition(field, { kind: (k) => k === 'get' || k === 'set' }) && + node.kind === 'get') || + node.kind === 'set' + ) { + throw generateError( + DecoratorErrors.SINGLE_DECORATOR_ON_SETTER_GETTER_PAIR, + node.key.name + ); + } else { + throw generateError(DecoratorErrors.DUPLICATE_API_PROPERTY, node.key.name); + } + } + } + switch (node.key.name) { case 'constructor': node.value.params = [b.identifier('propsAvailableAtConstruction')]; @@ -258,8 +286,8 @@ export default function compileJS( tmplExplicitImports: null, cssExplicitImports: null, staticStylesheetIds: null, - publicFields: [], - privateFields: [], + publicFields: new Map(), + privateFields: new Set(), wireAdapters: [], experimentalDynamicComponent: options.experimentalDynamicComponent, importManager: new ImportManager(), diff --git a/packages/@lwc/ssr-compiler/src/compile-js/types.ts b/packages/@lwc/ssr-compiler/src/compile-js/types.ts index 6ff7723f08..eaa17fc640 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/types.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/types.ts @@ -49,9 +49,9 @@ export interface ComponentMetaState { // the set of variable names associated with explicitly imported CSS files staticStylesheetIds: Set | null; // the public (`@api`-annotated) fields of the component class - publicFields: Array; + publicFields: Map; // the private fields of the component class - privateFields: Array; + privateFields: Set; // indicates whether the LightningElement has any wired props wireAdapters: WireAdapter[]; // dynamic imports configuration From 54192318a31a2c129c3979d7e97f06d17649eaff Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Fri, 20 Dec 2024 09:59:50 -0300 Subject: [PATCH 02/11] chore: simplify test --- .../src/__tests__/compilation.spec.ts | 44 ++----------------- 1 file changed, 3 insertions(+), 41 deletions(-) diff --git a/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts b/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts index 09c606e7a9..a7d2e8ed67 100644 --- a/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts +++ b/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts @@ -97,55 +97,17 @@ describe('component compilation errors', () => { }); describe('disallowed props', () => { - test('class', () => { + test.for(['class', 'is', 'slot', 'style'])('%s', (prop) => { expect( compile(/* js */ ` import { LightningElement, api } from 'lwc' export default class extends LightningElement { - @api class + @api ${prop} } `) ).toThrow( - `LWC1110: Invalid property name "class". "class" is a reserved attribute.` - ); - }); - - test('is', () => { - expect( - compile(/* js */ ` - import { LightningElement, api } from 'lwc' - - export default class extends LightningElement { - @api is - } - `) - ).toThrow(`LWC1110: Invalid property name "is". "is" is a reserved attribute.`); - }); - - test('slot', () => { - expect( - compile(/* js */ ` - import { LightningElement, api } from 'lwc' - - export default class extends LightningElement { - @api slot - } - `) - ).toThrow(`LWC1110: Invalid property name "slot". "slot" is a reserved attribute.`); - }); - - test('style', () => { - expect( - compile(/* js */ ` - import { LightningElement, api } from 'lwc' - - export default class extends LightningElement { - @api style - } - `) - ).toThrow( - `LWC1110: Invalid property name "style". "style" is a reserved attribute.` + `LWC1110: Invalid property name "${prop}". "${prop}" is a reserved attribute.` ); }); }); From 5049a6c1e2ea6fefe4c52635f8b88e2c04a92c7d Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Fri, 20 Dec 2024 12:50:35 -0300 Subject: [PATCH 03/11] feat: validate all api decorator errors --- .../src/__tests__/compilation.spec.ts | 164 +++++++++++++++--- .../@lwc/ssr-compiler/src/compile-js/index.ts | 70 +++++++- 2 files changed, 198 insertions(+), 36 deletions(-) diff --git a/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts b/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts index a7d2e8ed67..32a351be7b 100644 --- a/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts +++ b/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts @@ -39,11 +39,23 @@ describe('component compilation', () => { }); }); -describe('component compilation errors', () => { - describe('api decorator', () => { - test('conflicting api properties with getter/setter', () => { - expect( - compile(/* js */ ` +describe('api decorator', () => { + test('throws when combined with @track', () => { + expect( + compile(/* js */ ` + import { api, track, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @track + @api + apiWithTrack = "foo"; + } + `) + ).toThrow(`LWC1093: @api method or property cannot be used with @track`); + }); + + test('conflicting api properties with getter/setter', () => { + expect( + compile(/* js */ ` import { api, LightningElement } from "lwc"; export default class Text extends LightningElement { @api foo = 1; @@ -59,12 +71,12 @@ describe('component compilation errors', () => { } } `) - ).toThrow(`LWC1096: Duplicate @api property "foo".`); - }); + ).toThrow(`LWC1096: Duplicate @api property "foo".`); + }); - test('conflicting api properties with method', () => { - expect( - compile(/* js */ ` + test('conflicting api properties with method', () => { + expect( + compile(/* js */ ` import { api, LightningElement } from "lwc"; export default class Text extends LightningElement { @api foo = 1; @@ -73,12 +85,12 @@ describe('component compilation errors', () => { } } `) - ).toThrow(`LWC1096: Duplicate @api property "foo".`); - }); + ).toThrow(`LWC1096: Duplicate @api property "foo".`); + }); - test('detecting @api on both getter and a setter should produce an error', () => { - expect( - compile(/* js */ ` + test('detecting @api on both getter and a setter should produce an error', () => { + expect( + compile(/* js */ ` import { api, LightningElement } from "lwc"; export default class Test extends LightningElement { @api @@ -91,25 +103,123 @@ describe('component compilation errors', () => { } } `) - ).toThrow( - `LWC1112: @api get something and @api set something detected in class declaration. Only one of the two needs to be decorated with @api.` - ); - }); + ).toThrow( + `LWC1112: @api get something and @api set something detected in class declaration. Only one of the two needs to be decorated with @api.` + ); + }); - describe('disallowed props', () => { - test.for(['class', 'is', 'slot', 'style'])('%s', (prop) => { - expect( - compile(/* js */ ` + describe('disallowed props', () => { + test.for(['class', 'is', 'slot', 'style'])('%s', (prop) => { + expect( + compile(/* js */ ` import { LightningElement, api } from 'lwc' export default class extends LightningElement { @api ${prop} } `) - ).toThrow( - `LWC1110: Invalid property name "${prop}". "${prop}" is a reserved attribute.` - ); - }); + ).toThrow( + `LWC1110: Invalid property name "${prop}". "${prop}" is a reserved attribute.` + ); }); }); + + test('does not allow computed api getters and setters', () => { + expect( + compile(/* js */ ` + import { LightningElement, api } from "lwc"; + export default class ComputedAPIProp extends LightningElement { + @api + set [x](value) {} + get [x]() {} + } + `) + ).toThrow( + `LWC1106: @api cannot be applied to a computed property, getter, setter or method.` + ); + }); + + test('duplicate api properties', () => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Text extends LightningElement { + @api foo = 1; + @api foo = 2; + } + `) + ).toThrow(`LWC1096: Duplicate @api property "foo".`); + }); + + describe('throws if property name is ambiguous', () => { + test.for([ + ['bgcolor', 'bgColor'], + ['accesskey', 'accessKey'], + ['contenteditable', 'contentEditable'], + ['tabindex', 'tabIndex'], + ['maxlength', 'maxLength'], + ['maxvalue', 'maxValue'], + ] as const)('%s', ([prop, suggestion]) => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @api ${prop}; + } + `) + ).toThrow( + `LWC1109: Ambiguous attribute name "${prop}". "${prop}" will never be called from template because its corresponding property is camel cased. Consider renaming to "${suggestion}"` + ); + }); + }); + + test('throws error if default value is true', () => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @api publicProp = true; + } + `) + ).toThrow(`LWC1099: Boolean public property must default to false.`); + }); + + test('throws if property name is part', () => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @api part; + } + `) + ).toThrow( + `LWC1111: Invalid property name "part". "part" is a future reserved attribute for web components.` + ); + }); + + test('throws if property name prefixed with data', () => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @api dataFooBar; + } + `) + ).toThrow( + `LWC1107: Invalid property name "dataFooBar". Properties starting with "data" are reserved attributes.` + ); + }); + + test('throws if property name prefixed with on', () => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @api onChangeHandler; + } + `) + ).toThrow( + `LWC1108: Invalid property name "onChangeHandler". Properties starting with "on" are reserved for event handlers` + ); + }); }); diff --git a/packages/@lwc/ssr-compiler/src/compile-js/index.ts b/packages/@lwc/ssr-compiler/src/compile-js/index.ts index 389dca0e1c..85decd0eb3 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/index.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/index.ts @@ -25,10 +25,23 @@ import type { Identifier as EsIdentifier, Program as EsProgram, Decorator as EsDecorator, + MethodDefinition as EsMethodDefinition, + PropertyDefinition as EsPropertyDefinition, } from 'estree'; import type { Visitors, ComponentMetaState } from './types'; import type { CompilationMode } from '@lwc/shared'; +const DISALLOWED_PROP_SET = new Set(['is', 'class', 'slot', 'style']); + +const AMBIGUOUS_PROP_SET = new Map([ + ['bgcolor', 'bgColor'], + ['accesskey', 'accessKey'], + ['contenteditable', 'contentEditable'], + ['tabindex', 'tabIndex'], + ['maxlength', 'maxLength'], + ['maxvalue', 'maxValue'], +]); + const visitors: Visitors = { $: { scope: true }, ExportNamedDeclaration(path) { @@ -104,11 +117,8 @@ const visitors: Visitors = { if (state.publicFields.has(node.key.name)) { throw generateError(DecoratorErrors.DUPLICATE_API_PROPERTY, node.key.name); } - - if (['class', 'is', 'slot', 'style'].includes(node.key.name)) { - throw generateError(DecoratorErrors.PROPERTY_NAME_IS_RESERVED, node.key.name); - } - + validatePropertyName(node); + validatePropertyValue(node); state.publicFields.set(node.key.name, node); } else if ( is.callExpression(decoratedExpression) && @@ -176,9 +186,8 @@ const visitors: Visitors = { if (is.identifier(decoratedExpression, { name: 'api' })) { const field = state.publicFields.get(node.key.name); - if (!field) { - state.publicFields.set(node.key.name, node); - } else { + + if (field) { if ( (is.methodDefinition(field, { kind: (k) => k === 'get' || k === 'set' }) && node.kind === 'get') || @@ -192,6 +201,9 @@ const visitors: Visitors = { throw generateError(DecoratorErrors.DUPLICATE_API_PROPERTY, node.key.name); } } + + validatePropertyName(node); + state.publicFields.set(node.key.name, node); } switch (node.key.name) { @@ -257,9 +269,13 @@ function validateUniqueDecorator(decorators: EsDecorator[]) { const hasTrack = expressions.some((expr) => is.identifier(expr, { name: 'track' })); - if ((hasWire || hasApi) && hasTrack) { + if (hasWire && hasTrack) { throw generateError(DecoratorErrors.CONFLICT_WITH_ANOTHER_DECORATOR, 'track'); } + + if (hasApi && hasTrack) { + throw generateError(DecoratorErrors.API_AND_TRACK_DECORATOR_CONFLICT); + } } export default function compileJS( @@ -314,3 +330,39 @@ export default function compileJS( code: generate(ast, {}), }; } + +function isBooleanPropDefaultTrue(property: EsPropertyDefinition) { + const propertyValue = property.value; + return propertyValue && propertyValue.type === 'Literal' && propertyValue.value === true; +} + +function validatePropertyValue(property: EsPropertyDefinition) { + if (isBooleanPropDefaultTrue(property)) { + throw generateError(DecoratorErrors.INVALID_BOOLEAN_PUBLIC_PROPERTY); + } +} + +function validatePropertyName(property: EsMethodDefinition | EsPropertyDefinition) { + if (property.computed || !('name' in property.key)) { + throw generateError(DecoratorErrors.PROPERTY_CANNOT_BE_COMPUTED); + } + + const propertyName = property.key.name; + + switch (true) { + case propertyName === 'part': + throw generateError(DecoratorErrors.PROPERTY_NAME_PART_IS_RESERVED, propertyName); + case propertyName.startsWith('on'): + throw generateError(DecoratorErrors.PROPERTY_NAME_CANNOT_START_WITH_ON, propertyName); + case propertyName.startsWith('data') && propertyName.length > 4: + throw generateError(DecoratorErrors.PROPERTY_NAME_CANNOT_START_WITH_DATA, propertyName); + case DISALLOWED_PROP_SET.has(propertyName): + throw generateError(DecoratorErrors.PROPERTY_NAME_IS_RESERVED, propertyName); + case AMBIGUOUS_PROP_SET.has(propertyName): + throw generateError( + DecoratorErrors.PROPERTY_NAME_IS_AMBIGUOUS, + propertyName, + AMBIGUOUS_PROP_SET.get(propertyName)! + ); + } +} From 0409713a8e545642885b942075a9a9ef8a538975 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Fri, 20 Dec 2024 13:19:07 -0300 Subject: [PATCH 04/11] chore: move api-decorator tests to its own file --- .../src/__tests__/api-decorator.spec.ts | 183 +++++++++++++++++ .../src/__tests__/compilation.spec.ts | 191 ------------------ 2 files changed, 183 insertions(+), 191 deletions(-) create mode 100644 packages/@lwc/ssr-compiler/src/__tests__/api-decorator.spec.ts diff --git a/packages/@lwc/ssr-compiler/src/__tests__/api-decorator.spec.ts b/packages/@lwc/ssr-compiler/src/__tests__/api-decorator.spec.ts new file mode 100644 index 0000000000..594e450f1a --- /dev/null +++ b/packages/@lwc/ssr-compiler/src/__tests__/api-decorator.spec.ts @@ -0,0 +1,183 @@ +import { describe, test, expect } from 'vitest'; +import { compileComponentForSSR } from '../index'; + +const compile = + (src: string, filename = 'test.js') => + () => { + return compileComponentForSSR(src, filename, {}); + }; + +describe('thows error', () => { + test('combined with @track', () => { + expect( + compile(/* js */ ` + import { api, track, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @track + @api + apiWithTrack = "foo"; + } + `) + ).toThrow(`LWC1093: @api method or property cannot be used with @track`); + }); + + describe('conflicting api properties', () => { + test('getter/setter', () => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Text extends LightningElement { + @api foo = 1; + + _internal = 1; + + @api + get foo() { + return "foo"; + } + set foo(val) { + this._internal = val; + } + } + `) + ).toThrow(`LWC1096: Duplicate @api property "foo".`); + }); + + test('method', () => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Text extends LightningElement { + @api foo = 1; + @api foo() { + return "foo"; + } + } + `) + ).toThrow(`LWC1096: Duplicate @api property "foo".`); + }); + }); + + test('default value is true', () => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @api publicProp = true; + } + `) + ).toThrow(`LWC1099: Boolean public property must default to false.`); + }); + + test('computed api getters and setters', () => { + expect( + compile(/* js */ ` + import { LightningElement, api } from "lwc"; + export default class ComputedAPIProp extends LightningElement { + @api + set [x](value) {} + get [x]() {} + } + `) + ).toThrow( + `LWC1106: @api cannot be applied to a computed property, getter, setter or method.` + ); + }); + + test('property name prefixed with data', () => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @api dataFooBar; + } + `) + ).toThrow( + `LWC1107: Invalid property name "dataFooBar". Properties starting with "data" are reserved attributes.` + ); + }); + + test('property name prefixed with on', () => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @api onChangeHandler; + } + `) + ).toThrow( + `LWC1108: Invalid property name "onChangeHandler". Properties starting with "on" are reserved for event handlers` + ); + }); + + describe('property name is ambiguous', () => { + test.for([ + ['bgcolor', 'bgColor'], + ['accesskey', 'accessKey'], + ['contenteditable', 'contentEditable'], + ['tabindex', 'tabIndex'], + ['maxlength', 'maxLength'], + ['maxvalue', 'maxValue'], + ] as const)('%s', ([prop, suggestion]) => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @api ${prop}; + } + `) + ).toThrow( + `LWC1109: Ambiguous attribute name "${prop}". "${prop}" will never be called from template because its corresponding property is camel cased. Consider renaming to "${suggestion}"` + ); + }); + }); + + describe('disallowed props', () => { + test.for(['class', 'is', 'slot', 'style'])('%s', (prop) => { + expect( + compile(/* js */ ` + import { LightningElement, api } from 'lwc' + + export default class extends LightningElement { + @api ${prop} + } + `) + ).toThrow( + `LWC1110: Invalid property name "${prop}". "${prop}" is a reserved attribute.` + ); + }); + }); + + test('property name is part', () => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @api part; + } + `) + ).toThrow( + `LWC1111: Invalid property name "part". "part" is a future reserved attribute for web components.` + ); + }); + + test('both getter and a setter', () => { + expect( + compile(/* js */ ` + import { api, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @api + get something() { + return this.s; + } + @api + set something(value) { + this.s = value; + } + } + `) + ).toThrow( + `LWC1112: @api get something and @api set something detected in class declaration. Only one of the two needs to be decorated with @api.` + ); + }); +}); diff --git a/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts b/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts index 32a351be7b..95e75a688b 100644 --- a/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts +++ b/packages/@lwc/ssr-compiler/src/__tests__/compilation.spec.ts @@ -2,12 +2,6 @@ import path from 'node:path'; import { describe, test, expect } from 'vitest'; import { compileComponentForSSR } from '../index'; -const compile = - (src: string, filename = 'test.js') => - () => { - return compileComponentForSSR(src, filename, {}); - }; - describe('component compilation', () => { test('implicit templates imports do not use full file paths', () => { const src = ` @@ -38,188 +32,3 @@ describe('component compilation', () => { expect(code).toContain('import tmpl from "./component.html"'); }); }); - -describe('api decorator', () => { - test('throws when combined with @track', () => { - expect( - compile(/* js */ ` - import { api, track, LightningElement } from "lwc"; - export default class Test extends LightningElement { - @track - @api - apiWithTrack = "foo"; - } - `) - ).toThrow(`LWC1093: @api method or property cannot be used with @track`); - }); - - test('conflicting api properties with getter/setter', () => { - expect( - compile(/* js */ ` - import { api, LightningElement } from "lwc"; - export default class Text extends LightningElement { - @api foo = 1; - - _internal = 1; - - @api - get foo() { - return "foo"; - } - set foo(val) { - this._internal = val; - } - } - `) - ).toThrow(`LWC1096: Duplicate @api property "foo".`); - }); - - test('conflicting api properties with method', () => { - expect( - compile(/* js */ ` - import { api, LightningElement } from "lwc"; - export default class Text extends LightningElement { - @api foo = 1; - @api foo() { - return "foo"; - } - } - `) - ).toThrow(`LWC1096: Duplicate @api property "foo".`); - }); - - test('detecting @api on both getter and a setter should produce an error', () => { - expect( - compile(/* js */ ` - import { api, LightningElement } from "lwc"; - export default class Test extends LightningElement { - @api - get something() { - return this.s; - } - @api - set something(value) { - this.s = value; - } - } - `) - ).toThrow( - `LWC1112: @api get something and @api set something detected in class declaration. Only one of the two needs to be decorated with @api.` - ); - }); - - describe('disallowed props', () => { - test.for(['class', 'is', 'slot', 'style'])('%s', (prop) => { - expect( - compile(/* js */ ` - import { LightningElement, api } from 'lwc' - - export default class extends LightningElement { - @api ${prop} - } - `) - ).toThrow( - `LWC1110: Invalid property name "${prop}". "${prop}" is a reserved attribute.` - ); - }); - }); - - test('does not allow computed api getters and setters', () => { - expect( - compile(/* js */ ` - import { LightningElement, api } from "lwc"; - export default class ComputedAPIProp extends LightningElement { - @api - set [x](value) {} - get [x]() {} - } - `) - ).toThrow( - `LWC1106: @api cannot be applied to a computed property, getter, setter or method.` - ); - }); - - test('duplicate api properties', () => { - expect( - compile(/* js */ ` - import { api, LightningElement } from "lwc"; - export default class Text extends LightningElement { - @api foo = 1; - @api foo = 2; - } - `) - ).toThrow(`LWC1096: Duplicate @api property "foo".`); - }); - - describe('throws if property name is ambiguous', () => { - test.for([ - ['bgcolor', 'bgColor'], - ['accesskey', 'accessKey'], - ['contenteditable', 'contentEditable'], - ['tabindex', 'tabIndex'], - ['maxlength', 'maxLength'], - ['maxvalue', 'maxValue'], - ] as const)('%s', ([prop, suggestion]) => { - expect( - compile(/* js */ ` - import { api, LightningElement } from "lwc"; - export default class Test extends LightningElement { - @api ${prop}; - } - `) - ).toThrow( - `LWC1109: Ambiguous attribute name "${prop}". "${prop}" will never be called from template because its corresponding property is camel cased. Consider renaming to "${suggestion}"` - ); - }); - }); - - test('throws error if default value is true', () => { - expect( - compile(/* js */ ` - import { api, LightningElement } from "lwc"; - export default class Test extends LightningElement { - @api publicProp = true; - } - `) - ).toThrow(`LWC1099: Boolean public property must default to false.`); - }); - - test('throws if property name is part', () => { - expect( - compile(/* js */ ` - import { api, LightningElement } from "lwc"; - export default class Test extends LightningElement { - @api part; - } - `) - ).toThrow( - `LWC1111: Invalid property name "part". "part" is a future reserved attribute for web components.` - ); - }); - - test('throws if property name prefixed with data', () => { - expect( - compile(/* js */ ` - import { api, LightningElement } from "lwc"; - export default class Test extends LightningElement { - @api dataFooBar; - } - `) - ).toThrow( - `LWC1107: Invalid property name "dataFooBar". Properties starting with "data" are reserved attributes.` - ); - }); - - test('throws if property name prefixed with on', () => { - expect( - compile(/* js */ ` - import { api, LightningElement } from "lwc"; - export default class Test extends LightningElement { - @api onChangeHandler; - } - `) - ).toThrow( - `LWC1108: Invalid property name "onChangeHandler". Properties starting with "on" are reserved for event handlers` - ); - }); -}); From 84047ff6c6834a1a818a20650ea52807104268a4 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Fri, 20 Dec 2024 13:58:42 -0300 Subject: [PATCH 05/11] chore: format test --- .../src/__tests__/api-decorator.spec.ts | 177 ++++++++---------- 1 file changed, 81 insertions(+), 96 deletions(-) diff --git a/packages/@lwc/ssr-compiler/src/__tests__/api-decorator.spec.ts b/packages/@lwc/ssr-compiler/src/__tests__/api-decorator.spec.ts index 594e450f1a..9c13c7b1a7 100644 --- a/packages/@lwc/ssr-compiler/src/__tests__/api-decorator.spec.ts +++ b/packages/@lwc/ssr-compiler/src/__tests__/api-decorator.spec.ts @@ -9,103 +9,95 @@ const compile = describe('thows error', () => { test('combined with @track', () => { - expect( - compile(/* js */ ` - import { api, track, LightningElement } from "lwc"; - export default class Test extends LightningElement { - @track - @api - apiWithTrack = "foo"; - } - `) - ).toThrow(`LWC1093: @api method or property cannot be used with @track`); + const src = /* js */ ` + import { api, track, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @track + @api + apiWithTrack = "foo"; + } + `; + expect(compile(src)).toThrow(`LWC1093: @api method or property cannot be used with @track`); }); describe('conflicting api properties', () => { - test('getter/setter', () => { - expect( - compile(/* js */ ` - import { api, LightningElement } from "lwc"; - export default class Text extends LightningElement { - @api foo = 1; - - _internal = 1; - - @api - get foo() { - return "foo"; - } - set foo(val) { - this._internal = val; - } - } - `) - ).toThrow(`LWC1096: Duplicate @api property "foo".`); - }); - - test('method', () => { - expect( - compile(/* js */ ` - import { api, LightningElement } from "lwc"; - export default class Text extends LightningElement { - @api foo = 1; - @api foo() { - return "foo"; - } - } - `) - ).toThrow(`LWC1096: Duplicate @api property "foo".`); + test.for([ + [ + 'getter/setter', + /* js */ ` + @api foo = 1; + _internal = 1; + @api + get foo() { + return "foo"; + } + set foo(val) { + this._internal = val; + }`, + ], + [ + 'method', + /* js */ ` + @api foo = 1; + @api foo() { + return "foo"; + }`, + ], + ] as [name: string, body: string][])(`%s`, ([, body]) => { + const src = /* js */ ` + import { api, LightningElement } from "lwc"; + export default class Test extends LightningElement { + ${body} + } + `; + expect(compile(src)).toThrow(`LWC1096: Duplicate @api property "foo".`); }); }); test('default value is true', () => { - expect( - compile(/* js */ ` + const src = /* js */ ` import { api, LightningElement } from "lwc"; - export default class Test extends LightningElement { + export default class Test extends LightningElement { @api publicProp = true; } - `) - ).toThrow(`LWC1099: Boolean public property must default to false.`); + `; + expect(compile(src)).toThrow(`LWC1099: Boolean public property must default to false.`); }); test('computed api getters and setters', () => { - expect( - compile(/* js */ ` - import { LightningElement, api } from "lwc"; - export default class ComputedAPIProp extends LightningElement { + const src = /* js */ ` + import { api, LightningElement } from "lwc"; + export default class Test extends LightningElement { @api set [x](value) {} get [x]() {} } - `) - ).toThrow( + `; + expect(compile(src)).toThrow( `LWC1106: @api cannot be applied to a computed property, getter, setter or method.` ); }); test('property name prefixed with data', () => { - expect( - compile(/* js */ ` + const src = /* js */ ` import { api, LightningElement } from "lwc"; export default class Test extends LightningElement { @api dataFooBar; } - `) - ).toThrow( + `; + expect(compile(src)).toThrow( `LWC1107: Invalid property name "dataFooBar". Properties starting with "data" are reserved attributes.` ); }); test('property name prefixed with on', () => { - expect( - compile(/* js */ ` + const src = /* js */ ` import { api, LightningElement } from "lwc"; export default class Test extends LightningElement { @api onChangeHandler; } - `) - ).toThrow( + `; + expect(compile(src)).toThrow( `LWC1108: Invalid property name "onChangeHandler". Properties starting with "on" are reserved for event handlers` ); }); @@ -118,15 +110,14 @@ describe('thows error', () => { ['tabindex', 'tabIndex'], ['maxlength', 'maxLength'], ['maxvalue', 'maxValue'], - ] as const)('%s', ([prop, suggestion]) => { - expect( - compile(/* js */ ` + ] as [prop: string, suggestion: string][])('%s', ([prop, suggestion]) => { + const src = /* js */ ` import { api, LightningElement } from "lwc"; - export default class Test extends LightningElement { + export default class Test extends LightningElement { @api ${prop}; } - `) - ).toThrow( + `; + expect(compile(src)).toThrow( `LWC1109: Ambiguous attribute name "${prop}". "${prop}" will never be called from template because its corresponding property is camel cased. Consider renaming to "${suggestion}"` ); }); @@ -134,49 +125,43 @@ describe('thows error', () => { describe('disallowed props', () => { test.for(['class', 'is', 'slot', 'style'])('%s', (prop) => { - expect( - compile(/* js */ ` - import { LightningElement, api } from 'lwc' - - export default class extends LightningElement { - @api ${prop} - } - `) - ).toThrow( + const src = /* js */ ` + import { api, LightningElement } from 'lwc' + export default class Test extends LightningElement { + @api ${prop} + } + `; + expect(compile(src)).toThrow( `LWC1110: Invalid property name "${prop}". "${prop}" is a reserved attribute.` ); }); }); test('property name is part', () => { - expect( - compile(/* js */ ` + const src = /* js */ ` import { api, LightningElement } from "lwc"; - export default class Test extends LightningElement { + export default class Test extends LightningElement { @api part; } - `) - ).toThrow( + `; + expect(compile(src)).toThrow( `LWC1111: Invalid property name "part". "part" is a future reserved attribute for web components.` ); }); test('both getter and a setter', () => { - expect( - compile(/* js */ ` - import { api, LightningElement } from "lwc"; - export default class Test extends LightningElement { - @api - get something() { - return this.s; - } - @api - set something(value) { - this.s = value; - } + const src = /* js */ ` + import { api, LightningElement } from "lwc"; + export default class Test extends LightningElement { + @api get something() { + return this.s; + } + @api set something(value) { + this.s = value; } - `) - ).toThrow( + } + `; + expect(compile(src)).toThrow( `LWC1112: @api get something and @api set something detected in class declaration. Only one of the two needs to be decorated with @api.` ); }); From aaa8a2e3be26c1e5b763569ae92617f713b8cdc7 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Fri, 20 Dec 2024 16:21:59 -0300 Subject: [PATCH 06/11] chore: separate api decorator validation into its own file --- .../src/__tests__/api-decorator.spec.ts | 2 +- .../@lwc/ssr-compiler/src/compile-js/api.ts | 56 +++++++++++++++++++ .../@lwc/ssr-compiler/src/compile-js/index.ts | 51 +---------------- 3 files changed, 59 insertions(+), 50 deletions(-) create mode 100644 packages/@lwc/ssr-compiler/src/compile-js/api.ts diff --git a/packages/@lwc/ssr-compiler/src/__tests__/api-decorator.spec.ts b/packages/@lwc/ssr-compiler/src/__tests__/api-decorator.spec.ts index 9c13c7b1a7..dc03c08f75 100644 --- a/packages/@lwc/ssr-compiler/src/__tests__/api-decorator.spec.ts +++ b/packages/@lwc/ssr-compiler/src/__tests__/api-decorator.spec.ts @@ -43,7 +43,7 @@ describe('thows error', () => { return "foo"; }`, ], - ] as [name: string, body: string][])(`%s`, ([, body]) => { + ])(`%s`, ([, body]) => { const src = /* js */ ` import { api, LightningElement } from "lwc"; export default class Test extends LightningElement { diff --git a/packages/@lwc/ssr-compiler/src/compile-js/api.ts b/packages/@lwc/ssr-compiler/src/compile-js/api.ts new file mode 100644 index 0000000000..c15b582afa --- /dev/null +++ b/packages/@lwc/ssr-compiler/src/compile-js/api.ts @@ -0,0 +1,56 @@ +/* + * Copyright (c) 2024, Salesforce, Inc. + * All rights reserved. + * SPDX-License-Identifier: MIT + * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT + */ + +import { is } from 'estree-toolkit'; +import { DecoratorErrors } from '@lwc/errors'; +import { generateError } from './errors'; +import type { + MethodDefinition as EsMethodDefinition, + PropertyDefinition as EsPropertyDefinition, +} from 'estree'; + +const DISALLOWED_PROP_SET = new Set(['is', 'class', 'slot', 'style']); + +const AMBIGUOUS_PROP_SET = new Map([ + ['bgcolor', 'bgColor'], + ['accesskey', 'accessKey'], + ['contenteditable', 'contentEditable'], + ['tabindex', 'tabIndex'], + ['maxlength', 'maxLength'], + ['maxvalue', 'maxValue'], +]); + +export function validatePropertyValue(property: EsPropertyDefinition) { + if (is.literal(property.value, { value: true })) { + throw generateError(DecoratorErrors.INVALID_BOOLEAN_PUBLIC_PROPERTY); + } +} + +export function validatePropertyName(property: EsMethodDefinition | EsPropertyDefinition) { + if (property.computed || !('name' in property.key)) { + throw generateError(DecoratorErrors.PROPERTY_CANNOT_BE_COMPUTED); + } + + const propertyName = property.key.name; + + switch (true) { + case propertyName === 'part': + throw generateError(DecoratorErrors.PROPERTY_NAME_PART_IS_RESERVED, propertyName); + case propertyName.startsWith('on'): + throw generateError(DecoratorErrors.PROPERTY_NAME_CANNOT_START_WITH_ON, propertyName); + case propertyName.startsWith('data') && propertyName.length > 4: + throw generateError(DecoratorErrors.PROPERTY_NAME_CANNOT_START_WITH_DATA, propertyName); + case DISALLOWED_PROP_SET.has(propertyName): + throw generateError(DecoratorErrors.PROPERTY_NAME_IS_RESERVED, propertyName); + case AMBIGUOUS_PROP_SET.has(propertyName): + throw generateError( + DecoratorErrors.PROPERTY_NAME_IS_AMBIGUOUS, + propertyName, + AMBIGUOUS_PROP_SET.get(propertyName)! + ); + } +} diff --git a/packages/@lwc/ssr-compiler/src/compile-js/index.ts b/packages/@lwc/ssr-compiler/src/compile-js/index.ts index 85decd0eb3..63f91b464c 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/index.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/index.ts @@ -17,31 +17,20 @@ import { catalogTmplImport } from './catalog-tmpls'; import { catalogStaticStylesheets, catalogAndReplaceStyleImports } from './stylesheets'; import { addGenerateMarkupFunction } from './generate-markup'; import { catalogWireAdapters } from './wire'; +import { validatePropertyName, validatePropertyValue } from './api'; import { removeDecoratorImport } from './remove-decorator-import'; import { generateError } from './errors'; + import type { ComponentTransformOptions } from '../shared'; import type { Identifier as EsIdentifier, Program as EsProgram, Decorator as EsDecorator, - MethodDefinition as EsMethodDefinition, - PropertyDefinition as EsPropertyDefinition, } from 'estree'; import type { Visitors, ComponentMetaState } from './types'; import type { CompilationMode } from '@lwc/shared'; -const DISALLOWED_PROP_SET = new Set(['is', 'class', 'slot', 'style']); - -const AMBIGUOUS_PROP_SET = new Map([ - ['bgcolor', 'bgColor'], - ['accesskey', 'accessKey'], - ['contenteditable', 'contentEditable'], - ['tabindex', 'tabIndex'], - ['maxlength', 'maxLength'], - ['maxvalue', 'maxValue'], -]); - const visitors: Visitors = { $: { scope: true }, ExportNamedDeclaration(path) { @@ -330,39 +319,3 @@ export default function compileJS( code: generate(ast, {}), }; } - -function isBooleanPropDefaultTrue(property: EsPropertyDefinition) { - const propertyValue = property.value; - return propertyValue && propertyValue.type === 'Literal' && propertyValue.value === true; -} - -function validatePropertyValue(property: EsPropertyDefinition) { - if (isBooleanPropDefaultTrue(property)) { - throw generateError(DecoratorErrors.INVALID_BOOLEAN_PUBLIC_PROPERTY); - } -} - -function validatePropertyName(property: EsMethodDefinition | EsPropertyDefinition) { - if (property.computed || !('name' in property.key)) { - throw generateError(DecoratorErrors.PROPERTY_CANNOT_BE_COMPUTED); - } - - const propertyName = property.key.name; - - switch (true) { - case propertyName === 'part': - throw generateError(DecoratorErrors.PROPERTY_NAME_PART_IS_RESERVED, propertyName); - case propertyName.startsWith('on'): - throw generateError(DecoratorErrors.PROPERTY_NAME_CANNOT_START_WITH_ON, propertyName); - case propertyName.startsWith('data') && propertyName.length > 4: - throw generateError(DecoratorErrors.PROPERTY_NAME_CANNOT_START_WITH_DATA, propertyName); - case DISALLOWED_PROP_SET.has(propertyName): - throw generateError(DecoratorErrors.PROPERTY_NAME_IS_RESERVED, propertyName); - case AMBIGUOUS_PROP_SET.has(propertyName): - throw generateError( - DecoratorErrors.PROPERTY_NAME_IS_AMBIGUOUS, - propertyName, - AMBIGUOUS_PROP_SET.get(propertyName)! - ); - } -} From 984f5ed4036268d712b2a8baf577d0306849c4f3 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Fri, 20 Dec 2024 18:16:24 -0300 Subject: [PATCH 07/11] chore: improve validation functions --- .../@lwc/ssr-compiler/src/compile-js/api.ts | 48 +++++++++-- .../@lwc/ssr-compiler/src/compile-js/index.ts | 86 ++++++------------- .../@lwc/ssr-compiler/src/compile-js/track.ts | 13 +++ .../@lwc/ssr-compiler/src/compile-js/types.ts | 17 +++- .../@lwc/ssr-compiler/src/compile-js/wire.ts | 14 +++ 5 files changed, 112 insertions(+), 66 deletions(-) create mode 100644 packages/@lwc/ssr-compiler/src/compile-js/track.ts diff --git a/packages/@lwc/ssr-compiler/src/compile-js/api.ts b/packages/@lwc/ssr-compiler/src/compile-js/api.ts index c15b582afa..c4957e8bca 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/api.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/api.ts @@ -8,10 +8,8 @@ import { is } from 'estree-toolkit'; import { DecoratorErrors } from '@lwc/errors'; import { generateError } from './errors'; -import type { - MethodDefinition as EsMethodDefinition, - PropertyDefinition as EsPropertyDefinition, -} from 'estree'; +import { isMethodKind, type ComponentMetaState } from './types'; +import type { MethodDefinition, PropertyDefinition, Identifier, Decorator } from 'estree'; const DISALLOWED_PROP_SET = new Set(['is', 'class', 'slot', 'style']); @@ -24,13 +22,13 @@ const AMBIGUOUS_PROP_SET = new Map([ ['maxvalue', 'maxValue'], ]); -export function validatePropertyValue(property: EsPropertyDefinition) { +export function validatePropertyValue(property: PropertyDefinition) { if (is.literal(property.value, { value: true })) { throw generateError(DecoratorErrors.INVALID_BOOLEAN_PUBLIC_PROPERTY); } } -export function validatePropertyName(property: EsMethodDefinition | EsPropertyDefinition) { +export function validatePropertyName(property: MethodDefinition | PropertyDefinition) { if (property.computed || !('name' in property.key)) { throw generateError(DecoratorErrors.PROPERTY_CANNOT_BE_COMPUTED); } @@ -54,3 +52,41 @@ export function validatePropertyName(property: EsMethodDefinition | EsPropertyDe ); } } + +export function validateUniqueProperty( + node: PropertyDefinition & { key: Identifier }, + state: ComponentMetaState +) { + if (state.publicFields.has(node.key.name)) { + throw generateError(DecoratorErrors.DUPLICATE_API_PROPERTY, node.key.name); + } +} + +export default function validate( + node: (MethodDefinition & { key: Identifier }) | (PropertyDefinition & { key: Identifier }), + state: ComponentMetaState +) { + const field = state.publicFields.get(node.key.name); + + if (field) { + if ( + is.methodDefinition(field) && + isMethodKind(field, ['get', 'set']) && + is.methodDefinition(node) && + isMethodKind(node, ['get', 'set']) + ) { + throw generateError( + DecoratorErrors.SINGLE_DECORATOR_ON_SETTER_GETTER_PAIR, + node.key.name + ); + } + + throw generateError(DecoratorErrors.DUPLICATE_API_PROPERTY, node.key.name); + } +} + +export function isApiDecorator( + decorator: Decorator | undefined +): decorator is Decorator & { expression: Identifier & { name: 'api' } } { + return is.identifier(decorator?.expression) && decorator.expression.name === 'api'; +} diff --git a/packages/@lwc/ssr-compiler/src/compile-js/index.ts b/packages/@lwc/ssr-compiler/src/compile-js/index.ts index 63f91b464c..22344ae3c7 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/index.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/index.ts @@ -16,19 +16,25 @@ import { replaceLwcImport, replaceNamedLwcExport, replaceAllLwcExport } from './ import { catalogTmplImport } from './catalog-tmpls'; import { catalogStaticStylesheets, catalogAndReplaceStyleImports } from './stylesheets'; import { addGenerateMarkupFunction } from './generate-markup'; -import { catalogWireAdapters } from './wire'; -import { validatePropertyName, validatePropertyValue } from './api'; +import { catalogWireAdapters, isWireDecorator } from './wire'; +import validateUniqueness, { + isApiDecorator, + validatePropertyName, + validatePropertyValue, + validateUniqueProperty, +} from './api'; +import { isTrackDecorator } from './track'; import { removeDecoratorImport } from './remove-decorator-import'; import { generateError } from './errors'; +import { type Visitors, type ComponentMetaState, isKeyIdentifier, isMethodKind } from './types'; import type { ComponentTransformOptions } from '../shared'; import type { Identifier as EsIdentifier, Program as EsProgram, Decorator as EsDecorator, } from 'estree'; -import type { Visitors, ComponentMetaState } from './types'; import type { CompilationMode } from '@lwc/shared'; const visitors: Visitors = { @@ -95,25 +101,18 @@ const visitors: Visitors = { }, PropertyDefinition(path, state) { const node = path.node; - if (!is.identifier(node?.key)) { + if (!isKeyIdentifier(node)) { return; } const { decorators } = node; validateUniqueDecorator(decorators); - const decoratedExpression = decorators?.[0]?.expression; - if (is.identifier(decoratedExpression) && decoratedExpression.name === 'api') { - if (state.publicFields.has(node.key.name)) { - throw generateError(DecoratorErrors.DUPLICATE_API_PROPERTY, node.key.name); - } + if (isApiDecorator(decorators[0])) { + validateUniqueProperty(node, state); validatePropertyName(node); validatePropertyValue(node); state.publicFields.set(node.key.name, node); - } else if ( - is.callExpression(decoratedExpression) && - is.identifier(decoratedExpression.callee) && - decoratedExpression.callee.name === 'wire' - ) { + } else if (isWireDecorator(decorators[0])) { catalogWireAdapters(path, state); state.privateFields.add(node.key.name); } else { @@ -134,10 +133,9 @@ const visitors: Visitors = { }, MethodDefinition(path, state) { const node = path.node; - if (!is.identifier(node?.key)) { + if (!isKeyIdentifier(node)) { return; } - // If we mutate any class-methods that are piped through this compiler, then we'll be // inadvertently mutating things like Wire adapters. if (!state.isLWC) { @@ -146,16 +144,10 @@ const visitors: Visitors = { const { decorators } = node; validateUniqueDecorator(decorators); - // The real type is a subset of `Expression`, which doesn't work with the `is` validators - const decoratedExpression = decorators?.[0]?.expression; - if ( - is.callExpression(decoratedExpression) && - is.identifier(decoratedExpression.callee) && - decoratedExpression.callee.name === 'wire' - ) { + if (isWireDecorator(decorators[0])) { // Getters and setters are methods in the AST, but treated as properties by @wire // Note that this means that their implementations are ignored! - if (node.kind === 'get' || node.kind === 'set') { + if (isMethodKind(node, ['get', 'set'])) { const methodAsProp = b.propertyDefinition( structuredClone(node.key), null, @@ -171,26 +163,8 @@ const visitors: Visitors = { } else { catalogWireAdapters(path, state); } - } - - if (is.identifier(decoratedExpression, { name: 'api' })) { - const field = state.publicFields.get(node.key.name); - - if (field) { - if ( - (is.methodDefinition(field, { kind: (k) => k === 'get' || k === 'set' }) && - node.kind === 'get') || - node.kind === 'set' - ) { - throw generateError( - DecoratorErrors.SINGLE_DECORATOR_ON_SETTER_GETTER_PAIR, - node.key.name - ); - } else { - throw generateError(DecoratorErrors.DUPLICATE_API_PROPERTY, node.key.name); - } - } - + } else if (isApiDecorator(decorators[0])) { + validateUniqueness(node, state); validatePropertyName(node); state.publicFields.set(node.key.name, node); } @@ -244,22 +218,18 @@ function validateUniqueDecorator(decorators: EsDecorator[]) { return; } - const expressions = decorators.map(({ expression }) => expression); - - const hasWire = expressions.some( - (expr) => is.callExpression(expr) && is.identifier(expr.callee, { name: 'wire' }) - ); - - const hasApi = expressions.some((expr) => is.identifier(expr, { name: 'api' })); + const hasWire = decorators.some(isWireDecorator); + const hasApi = decorators.some(isApiDecorator); + const hasTrack = decorators.some(isTrackDecorator); - if (hasWire && hasApi) { - throw generateError(DecoratorErrors.CONFLICT_WITH_ANOTHER_DECORATOR, 'api'); - } - - const hasTrack = expressions.some((expr) => is.identifier(expr, { name: 'track' })); + if (hasWire) { + if (hasApi) { + throw generateError(DecoratorErrors.CONFLICT_WITH_ANOTHER_DECORATOR, 'api'); + } - if (hasWire && hasTrack) { - throw generateError(DecoratorErrors.CONFLICT_WITH_ANOTHER_DECORATOR, 'track'); + if (hasTrack) { + throw generateError(DecoratorErrors.CONFLICT_WITH_ANOTHER_DECORATOR, 'track'); + } } if (hasApi && hasTrack) { diff --git a/packages/@lwc/ssr-compiler/src/compile-js/track.ts b/packages/@lwc/ssr-compiler/src/compile-js/track.ts new file mode 100644 index 0000000000..d72f9f4156 --- /dev/null +++ b/packages/@lwc/ssr-compiler/src/compile-js/track.ts @@ -0,0 +1,13 @@ +/* + * Copyright (c) 2024, Salesforce, Inc. + * All rights reserved. + * SPDX-License-Identifier: MIT + * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT + */ +import type { Decorator, Identifier } from 'estree'; + +export function isTrackDecorator( + decorator: Decorator | undefined +): decorator is Decorator & { expression: Identifier & { name: 'track' } } { + return decorator?.expression.type === 'Identifier' && decorator.expression.name === 'track'; +} diff --git a/packages/@lwc/ssr-compiler/src/compile-js/types.ts b/packages/@lwc/ssr-compiler/src/compile-js/types.ts index eaa17fc640..7d412eb819 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/types.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/types.ts @@ -5,9 +5,9 @@ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ +import { type traverse } from 'estree-toolkit'; import type { ImportManager } from '../imports'; import type { ComponentTransformOptions } from '../shared'; -import type { traverse } from 'estree-toolkit'; import type { Identifier, MemberExpression, @@ -25,6 +25,19 @@ export interface WireAdapter { field: MethodDefinition | PropertyDefinition; } +export function isMethodKind< + T extends MethodDefinition, + const K extends [T['kind'], ...T['kind'][]], +>(node: T, kind: K): node is T & { kind: K[number] } { + return kind.includes(node.kind); +} + +export function isKeyIdentifier( + node: T | undefined | null +): node is T & { key: Identifier } { + return node?.key.type === 'Identifier'; +} + export interface ComponentMetaState { // indicates whether the LightningElement subclass is found in the JS being traversed isLWC: boolean; @@ -49,7 +62,7 @@ export interface ComponentMetaState { // the set of variable names associated with explicitly imported CSS files staticStylesheetIds: Set | null; // the public (`@api`-annotated) fields of the component class - publicFields: Map; + publicFields: Map; // the private fields of the component class privateFields: Set; // indicates whether the LightningElement has any wired props diff --git a/packages/@lwc/ssr-compiler/src/compile-js/wire.ts b/packages/@lwc/ssr-compiler/src/compile-js/wire.ts index 189153e16e..64ee6ebf32 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/wire.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/wire.ts @@ -22,6 +22,8 @@ import type { MemberExpression, Property, BlockStatement, + Decorator, + CallExpression, } from 'estree'; import type { ComponentMetaState, WireAdapter } from './types'; @@ -208,3 +210,15 @@ export function bWireAdaptersPlumbing(adapters: WireAdapter[]): BlockStatement[] return bWireAdapterPlumbing(adapterId, actionUponNewValue, config); }); } + +export function isWireDecorator( + decorator: Decorator | undefined +): decorator is Decorator & { + expression: CallExpression & { callee: Identifier & { name: 'wire' } }; +} { + return ( + is.callExpression(decorator?.expression) && + is.identifier(decorator.expression.callee) && + decorator.expression.callee.name === 'wire' + ); +} From 20c222edb9d2e5235dadce6b276540bb35d5551a Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Fri, 20 Dec 2024 18:34:26 -0300 Subject: [PATCH 08/11] chore: encapsulate api validators --- .../@lwc/ssr-compiler/src/compile-js/api.ts | 27 ++++++++++++++++--- .../@lwc/ssr-compiler/src/compile-js/index.ts | 20 +++++--------- 2 files changed, 29 insertions(+), 18 deletions(-) diff --git a/packages/@lwc/ssr-compiler/src/compile-js/api.ts b/packages/@lwc/ssr-compiler/src/compile-js/api.ts index c4957e8bca..8e4c7c082c 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/api.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/api.ts @@ -28,8 +28,10 @@ export function validatePropertyValue(property: PropertyDefinition) { } } -export function validatePropertyName(property: MethodDefinition | PropertyDefinition) { - if (property.computed || !('name' in property.key)) { +export function validateName( + property: (MethodDefinition & { key: Identifier }) | (PropertyDefinition & { key: Identifier }) +) { + if (property.computed) { throw generateError(DecoratorErrors.PROPERTY_CANNOT_BE_COMPUTED); } @@ -62,8 +64,8 @@ export function validateUniqueProperty( } } -export default function validate( - node: (MethodDefinition & { key: Identifier }) | (PropertyDefinition & { key: Identifier }), +export function validateUniqueMethod( + node: MethodDefinition & { key: Identifier }, state: ComponentMetaState ) { const field = state.publicFields.get(node.key.name); @@ -85,6 +87,23 @@ export default function validate( } } +export function validateApiProperty( + node: PropertyDefinition & { key: Identifier }, + state: ComponentMetaState +) { + validateUniqueProperty(node, state); + validateName(node); + validatePropertyValue(node); +} + +export function validateApiMethod( + node: MethodDefinition & { key: Identifier }, + state: ComponentMetaState +) { + validateUniqueMethod(node, state); + validateName(node); +} + export function isApiDecorator( decorator: Decorator | undefined ): decorator is Decorator & { expression: Identifier & { name: 'api' } } { diff --git a/packages/@lwc/ssr-compiler/src/compile-js/index.ts b/packages/@lwc/ssr-compiler/src/compile-js/index.ts index 22344ae3c7..51d6b036f4 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/index.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/index.ts @@ -17,12 +17,7 @@ import { catalogTmplImport } from './catalog-tmpls'; import { catalogStaticStylesheets, catalogAndReplaceStyleImports } from './stylesheets'; import { addGenerateMarkupFunction } from './generate-markup'; import { catalogWireAdapters, isWireDecorator } from './wire'; -import validateUniqueness, { - isApiDecorator, - validatePropertyName, - validatePropertyValue, - validateUniqueProperty, -} from './api'; +import { isApiDecorator, validateApiProperty, validateApiMethod } from './api'; import { isTrackDecorator } from './track'; import { removeDecoratorImport } from './remove-decorator-import'; @@ -108,9 +103,7 @@ const visitors: Visitors = { const { decorators } = node; validateUniqueDecorator(decorators); if (isApiDecorator(decorators[0])) { - validateUniqueProperty(node, state); - validatePropertyName(node); - validatePropertyValue(node); + validateApiProperty(node, state); state.publicFields.set(node.key.name, node); } else if (isWireDecorator(decorators[0])) { catalogWireAdapters(path, state); @@ -144,7 +137,10 @@ const visitors: Visitors = { const { decorators } = node; validateUniqueDecorator(decorators); - if (isWireDecorator(decorators[0])) { + if (isApiDecorator(decorators[0])) { + validateApiMethod(node, state); + state.publicFields.set(node.key.name, node); + } else if (isWireDecorator(decorators[0])) { // Getters and setters are methods in the AST, but treated as properties by @wire // Note that this means that their implementations are ignored! if (isMethodKind(node, ['get', 'set'])) { @@ -163,10 +159,6 @@ const visitors: Visitors = { } else { catalogWireAdapters(path, state); } - } else if (isApiDecorator(decorators[0])) { - validateUniqueness(node, state); - validatePropertyName(node); - state.publicFields.set(node.key.name, node); } switch (node.key.name) { From e5af37c22c205ff5ee1ee8cae59acd7e29b88fa3 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Fri, 20 Dec 2024 19:16:06 -0300 Subject: [PATCH 09/11] chore: create api folder --- .../ssr-compiler/src/compile-js/api/consts.ts | 15 ++++ .../ssr-compiler/src/compile-js/api/index.ts | 17 ++++ .../ssr-compiler/src/compile-js/api/types.ts | 16 ++++ .../compile-js/{api.ts => api/validate.ts} | 78 ++++++------------- .../@lwc/ssr-compiler/src/compile-js/index.ts | 3 +- 5 files changed, 73 insertions(+), 56 deletions(-) create mode 100644 packages/@lwc/ssr-compiler/src/compile-js/api/consts.ts create mode 100644 packages/@lwc/ssr-compiler/src/compile-js/api/index.ts create mode 100644 packages/@lwc/ssr-compiler/src/compile-js/api/types.ts rename packages/@lwc/ssr-compiler/src/compile-js/{api.ts => api/validate.ts} (53%) diff --git a/packages/@lwc/ssr-compiler/src/compile-js/api/consts.ts b/packages/@lwc/ssr-compiler/src/compile-js/api/consts.ts new file mode 100644 index 0000000000..bfa76f9a18 --- /dev/null +++ b/packages/@lwc/ssr-compiler/src/compile-js/api/consts.ts @@ -0,0 +1,15 @@ +/* + * Copyright (c) 2024, Salesforce, Inc. + * All rights reserved. + * SPDX-License-Identifier: MIT + * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT + */ +export const DISALLOWED_PROP_SET = new Set(['is', 'class', 'slot', 'style']); +export const AMBIGUOUS_PROP_SET = new Map([ + ['bgcolor', 'bgColor'], + ['accesskey', 'accessKey'], + ['contenteditable', 'contentEditable'], + ['tabindex', 'tabIndex'], + ['maxlength', 'maxLength'], + ['maxvalue', 'maxValue'], +]); diff --git a/packages/@lwc/ssr-compiler/src/compile-js/api/index.ts b/packages/@lwc/ssr-compiler/src/compile-js/api/index.ts new file mode 100644 index 0000000000..06a0cfea44 --- /dev/null +++ b/packages/@lwc/ssr-compiler/src/compile-js/api/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright (c) 2024, Salesforce, Inc. + * All rights reserved. + * SPDX-License-Identifier: MIT + * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT + */ +import type { Decorator, Identifier } from 'estree'; + +type ApiDecorator = Decorator & { + expression: Identifier & { + name: 'api'; + }; +}; + +export function isApiDecorator(decorator: Decorator | undefined): decorator is ApiDecorator { + return decorator?.expression.type === 'Identifier' && decorator.expression.name === 'api'; +} diff --git a/packages/@lwc/ssr-compiler/src/compile-js/api/types.ts b/packages/@lwc/ssr-compiler/src/compile-js/api/types.ts new file mode 100644 index 0000000000..5a63dc85dd --- /dev/null +++ b/packages/@lwc/ssr-compiler/src/compile-js/api/types.ts @@ -0,0 +1,16 @@ +/* + * Copyright (c) 2024, Salesforce, Inc. + * All rights reserved. + * SPDX-License-Identifier: MIT + * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT + */ +import type { MethodDefinition, Identifier, PropertyDefinition } from 'estree'; + +export type ApiMethodDefinition = MethodDefinition & { + key: Identifier; +}; +export type ApiPropertyDefinition = PropertyDefinition & { + key: Identifier; +}; + +export type ApiDefinition = ApiPropertyDefinition | ApiMethodDefinition; diff --git a/packages/@lwc/ssr-compiler/src/compile-js/api.ts b/packages/@lwc/ssr-compiler/src/compile-js/api/validate.ts similarity index 53% rename from packages/@lwc/ssr-compiler/src/compile-js/api.ts rename to packages/@lwc/ssr-compiler/src/compile-js/api/validate.ts index 8e4c7c082c..d5bd5e5a09 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/api.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/api/validate.ts @@ -5,37 +5,18 @@ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ -import { is } from 'estree-toolkit'; import { DecoratorErrors } from '@lwc/errors'; -import { generateError } from './errors'; -import { isMethodKind, type ComponentMetaState } from './types'; -import type { MethodDefinition, PropertyDefinition, Identifier, Decorator } from 'estree'; +import { generateError } from '../errors'; +import { isMethodKind, type ComponentMetaState } from '../types'; +import { DISALLOWED_PROP_SET, AMBIGUOUS_PROP_SET } from './consts'; +import type { ApiPropertyDefinition, ApiDefinition, ApiMethodDefinition } from './types'; -const DISALLOWED_PROP_SET = new Set(['is', 'class', 'slot', 'style']); - -const AMBIGUOUS_PROP_SET = new Map([ - ['bgcolor', 'bgColor'], - ['accesskey', 'accessKey'], - ['contenteditable', 'contentEditable'], - ['tabindex', 'tabIndex'], - ['maxlength', 'maxLength'], - ['maxvalue', 'maxValue'], -]); - -export function validatePropertyValue(property: PropertyDefinition) { - if (is.literal(property.value, { value: true })) { - throw generateError(DecoratorErrors.INVALID_BOOLEAN_PUBLIC_PROPERTY); - } -} - -export function validateName( - property: (MethodDefinition & { key: Identifier }) | (PropertyDefinition & { key: Identifier }) -) { - if (property.computed) { +function validateName(definition: ApiDefinition) { + if (definition.computed) { throw generateError(DecoratorErrors.PROPERTY_CANNOT_BE_COMPUTED); } - const propertyName = property.key.name; + const propertyName = definition.key.name; switch (true) { case propertyName === 'part': @@ -55,26 +36,31 @@ export function validateName( } } -export function validateUniqueProperty( - node: PropertyDefinition & { key: Identifier }, - state: ComponentMetaState -) { +function validatePropertyValue(property: ApiPropertyDefinition) { + if (property.value && property.value.type === 'Literal' && property.value.value === true) { + throw generateError(DecoratorErrors.INVALID_BOOLEAN_PUBLIC_PROPERTY); + } +} + +function validatePropertyUnique(node: ApiPropertyDefinition, state: ComponentMetaState) { if (state.publicFields.has(node.key.name)) { throw generateError(DecoratorErrors.DUPLICATE_API_PROPERTY, node.key.name); } } -export function validateUniqueMethod( - node: MethodDefinition & { key: Identifier }, - state: ComponentMetaState -) { +export function validateApiProperty(node: ApiPropertyDefinition, state: ComponentMetaState) { + validatePropertyUnique(node, state); + validateName(node); + validatePropertyValue(node); +} + +function validateUniqueMethod(node: ApiMethodDefinition, state: ComponentMetaState) { const field = state.publicFields.get(node.key.name); if (field) { if ( - is.methodDefinition(field) && + field.type === 'MethodDefinition' && isMethodKind(field, ['get', 'set']) && - is.methodDefinition(node) && isMethodKind(node, ['get', 'set']) ) { throw generateError( @@ -87,25 +73,7 @@ export function validateUniqueMethod( } } -export function validateApiProperty( - node: PropertyDefinition & { key: Identifier }, - state: ComponentMetaState -) { - validateUniqueProperty(node, state); - validateName(node); - validatePropertyValue(node); -} - -export function validateApiMethod( - node: MethodDefinition & { key: Identifier }, - state: ComponentMetaState -) { +export function validateApiMethod(node: ApiMethodDefinition, state: ComponentMetaState) { validateUniqueMethod(node, state); validateName(node); } - -export function isApiDecorator( - decorator: Decorator | undefined -): decorator is Decorator & { expression: Identifier & { name: 'api' } } { - return is.identifier(decorator?.expression) && decorator.expression.name === 'api'; -} diff --git a/packages/@lwc/ssr-compiler/src/compile-js/index.ts b/packages/@lwc/ssr-compiler/src/compile-js/index.ts index 51d6b036f4..e6c8909a8d 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/index.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/index.ts @@ -17,7 +17,8 @@ import { catalogTmplImport } from './catalog-tmpls'; import { catalogStaticStylesheets, catalogAndReplaceStyleImports } from './stylesheets'; import { addGenerateMarkupFunction } from './generate-markup'; import { catalogWireAdapters, isWireDecorator } from './wire'; -import { isApiDecorator, validateApiProperty, validateApiMethod } from './api'; +import { validateApiProperty, validateApiMethod } from './api/validate'; +import { isApiDecorator } from './api'; import { isTrackDecorator } from './track'; import { removeDecoratorImport } from './remove-decorator-import'; From 2505c01181ee2688e57f8b0b2905dc99f4e095b8 Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Fri, 20 Dec 2024 20:15:52 -0300 Subject: [PATCH 10/11] chore: move decorators to their own folder --- .../ssr-compiler/src/compile-js/api/types.ts | 16 ------ .../compile-js/{ => decorators}/api/consts.ts | 0 .../compile-js/{ => decorators}/api/index.ts | 6 +-- .../{ => decorators}/api/validate.ts | 38 ++++++++------ .../src/compile-js/decorators/index.ts | 36 ++++++++++++++ .../src/compile-js/{ => decorators}/track.ts | 0 .../src/compile-js/{ => decorators}/wire.ts | 10 ++-- .../src/compile-js/generate-markup.ts | 2 +- .../@lwc/ssr-compiler/src/compile-js/index.ts | 49 ++++++------------- .../@lwc/ssr-compiler/src/compile-js/types.ts | 18 ++----- 10 files changed, 86 insertions(+), 89 deletions(-) delete mode 100644 packages/@lwc/ssr-compiler/src/compile-js/api/types.ts rename packages/@lwc/ssr-compiler/src/compile-js/{ => decorators}/api/consts.ts (100%) rename packages/@lwc/ssr-compiler/src/compile-js/{ => decorators}/api/index.ts (87%) rename packages/@lwc/ssr-compiler/src/compile-js/{ => decorators}/api/validate.ts (74%) create mode 100644 packages/@lwc/ssr-compiler/src/compile-js/decorators/index.ts rename packages/@lwc/ssr-compiler/src/compile-js/{ => decorators}/track.ts (100%) rename packages/@lwc/ssr-compiler/src/compile-js/{ => decorators}/wire.ts (97%) diff --git a/packages/@lwc/ssr-compiler/src/compile-js/api/types.ts b/packages/@lwc/ssr-compiler/src/compile-js/api/types.ts deleted file mode 100644 index 5a63dc85dd..0000000000 --- a/packages/@lwc/ssr-compiler/src/compile-js/api/types.ts +++ /dev/null @@ -1,16 +0,0 @@ -/* - * Copyright (c) 2024, Salesforce, Inc. - * All rights reserved. - * SPDX-License-Identifier: MIT - * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT - */ -import type { MethodDefinition, Identifier, PropertyDefinition } from 'estree'; - -export type ApiMethodDefinition = MethodDefinition & { - key: Identifier; -}; -export type ApiPropertyDefinition = PropertyDefinition & { - key: Identifier; -}; - -export type ApiDefinition = ApiPropertyDefinition | ApiMethodDefinition; diff --git a/packages/@lwc/ssr-compiler/src/compile-js/api/consts.ts b/packages/@lwc/ssr-compiler/src/compile-js/decorators/api/consts.ts similarity index 100% rename from packages/@lwc/ssr-compiler/src/compile-js/api/consts.ts rename to packages/@lwc/ssr-compiler/src/compile-js/decorators/api/consts.ts diff --git a/packages/@lwc/ssr-compiler/src/compile-js/api/index.ts b/packages/@lwc/ssr-compiler/src/compile-js/decorators/api/index.ts similarity index 87% rename from packages/@lwc/ssr-compiler/src/compile-js/api/index.ts rename to packages/@lwc/ssr-compiler/src/compile-js/decorators/api/index.ts index 06a0cfea44..bbb791b887 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/api/index.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/decorators/api/index.ts @@ -6,12 +6,10 @@ */ import type { Decorator, Identifier } from 'estree'; -type ApiDecorator = Decorator & { +export function isApiDecorator(decorator: Decorator | undefined): decorator is Decorator & { expression: Identifier & { name: 'api'; }; -}; - -export function isApiDecorator(decorator: Decorator | undefined): decorator is ApiDecorator { +} { return decorator?.expression.type === 'Identifier' && decorator.expression.name === 'api'; } diff --git a/packages/@lwc/ssr-compiler/src/compile-js/api/validate.ts b/packages/@lwc/ssr-compiler/src/compile-js/decorators/api/validate.ts similarity index 74% rename from packages/@lwc/ssr-compiler/src/compile-js/api/validate.ts rename to packages/@lwc/ssr-compiler/src/compile-js/decorators/api/validate.ts index d5bd5e5a09..f0f4badee3 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/api/validate.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/decorators/api/validate.ts @@ -6,10 +6,19 @@ */ import { DecoratorErrors } from '@lwc/errors'; -import { generateError } from '../errors'; -import { isMethodKind, type ComponentMetaState } from '../types'; +import { generateError } from '../../errors'; +import { type ComponentMetaState } from '../../types'; import { DISALLOWED_PROP_SET, AMBIGUOUS_PROP_SET } from './consts'; -import type { ApiPropertyDefinition, ApiDefinition, ApiMethodDefinition } from './types'; +import type { Identifier, MethodDefinition, PropertyDefinition } from 'estree'; + +export type ApiMethodDefinition = MethodDefinition & { + key: Identifier; +}; +export type ApiPropertyDefinition = PropertyDefinition & { + key: Identifier; +}; + +export type ApiDefinition = ApiPropertyDefinition | ApiMethodDefinition; function validateName(definition: ApiDefinition) { if (definition.computed) { @@ -57,20 +66,19 @@ export function validateApiProperty(node: ApiPropertyDefinition, state: Componen function validateUniqueMethod(node: ApiMethodDefinition, state: ComponentMetaState) { const field = state.publicFields.get(node.key.name); - if (field) { - if ( - field.type === 'MethodDefinition' && - isMethodKind(field, ['get', 'set']) && - isMethodKind(node, ['get', 'set']) - ) { - throw generateError( - DecoratorErrors.SINGLE_DECORATOR_ON_SETTER_GETTER_PAIR, - node.key.name - ); - } + if (!field) { + return; + } - throw generateError(DecoratorErrors.DUPLICATE_API_PROPERTY, node.key.name); + if ( + field.type === 'MethodDefinition' && + (field.kind === 'get' || field.kind === 'set') && + (node.kind === 'get' || node.kind === 'set') + ) { + throw generateError(DecoratorErrors.SINGLE_DECORATOR_ON_SETTER_GETTER_PAIR, node.key.name); } + + throw generateError(DecoratorErrors.DUPLICATE_API_PROPERTY, node.key.name); } export function validateApiMethod(node: ApiMethodDefinition, state: ComponentMetaState) { diff --git a/packages/@lwc/ssr-compiler/src/compile-js/decorators/index.ts b/packages/@lwc/ssr-compiler/src/compile-js/decorators/index.ts new file mode 100644 index 0000000000..b447caf45a --- /dev/null +++ b/packages/@lwc/ssr-compiler/src/compile-js/decorators/index.ts @@ -0,0 +1,36 @@ +/* + * Copyright (c) 2024, Salesforce, Inc. + * All rights reserved. + * SPDX-License-Identifier: MIT + * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT + */ +import { DecoratorErrors } from '@lwc/errors'; +import { generateError } from '../errors'; +import { isApiDecorator } from './api'; +import { isTrackDecorator } from './track'; +import { isWireDecorator } from './wire'; +import type { Decorator as EsDecorator } from 'estree'; + +export function validateUniqueDecorator(decorators: EsDecorator[]) { + if (decorators.length < 2) { + return; + } + + const hasWire = decorators.some(isWireDecorator); + const hasApi = decorators.some(isApiDecorator); + const hasTrack = decorators.some(isTrackDecorator); + + if (hasWire) { + if (hasApi) { + throw generateError(DecoratorErrors.CONFLICT_WITH_ANOTHER_DECORATOR, 'api'); + } + + if (hasTrack) { + throw generateError(DecoratorErrors.CONFLICT_WITH_ANOTHER_DECORATOR, 'track'); + } + } + + if (hasApi && hasTrack) { + throw generateError(DecoratorErrors.API_AND_TRACK_DECORATOR_CONFLICT); + } +} diff --git a/packages/@lwc/ssr-compiler/src/compile-js/track.ts b/packages/@lwc/ssr-compiler/src/compile-js/decorators/track.ts similarity index 100% rename from packages/@lwc/ssr-compiler/src/compile-js/track.ts rename to packages/@lwc/ssr-compiler/src/compile-js/decorators/track.ts diff --git a/packages/@lwc/ssr-compiler/src/compile-js/wire.ts b/packages/@lwc/ssr-compiler/src/compile-js/decorators/wire.ts similarity index 97% rename from packages/@lwc/ssr-compiler/src/compile-js/wire.ts rename to packages/@lwc/ssr-compiler/src/compile-js/decorators/wire.ts index 64ee6ebf32..f690a9030f 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/wire.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/decorators/wire.ts @@ -8,8 +8,8 @@ import { is, builders as b } from 'estree-toolkit'; import { produce } from 'immer'; import { DecoratorErrors } from '@lwc/errors'; -import { esTemplate } from '../estemplate'; -import { generateError } from './errors'; +import { esTemplate } from '../../estemplate'; +import { generateError } from '../errors'; import type { NodePath } from 'estree-toolkit'; import type { @@ -25,7 +25,7 @@ import type { Decorator, CallExpression, } from 'estree'; -import type { ComponentMetaState, WireAdapter } from './types'; +import type { ComponentMetaState, WireAdapter } from '../types'; interface NoSpreadObjectExpression extends Omit { properties: Property[]; @@ -211,9 +211,7 @@ export function bWireAdaptersPlumbing(adapters: WireAdapter[]): BlockStatement[] }); } -export function isWireDecorator( - decorator: Decorator | undefined -): decorator is Decorator & { +export function isWireDecorator(decorator: Decorator | undefined): decorator is Decorator & { expression: CallExpression & { callee: Identifier & { name: 'wire' } }; } { return ( diff --git a/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts b/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts index fb29f97ba1..23073babb8 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/generate-markup.ts @@ -9,7 +9,7 @@ import { parse as pathParse } from 'node:path'; import { is, builders as b } from 'estree-toolkit'; import { esTemplate } from '../estemplate'; import { bImportDeclaration } from '../estree/builders'; -import { bWireAdaptersPlumbing } from './wire'; +import { bWireAdaptersPlumbing } from './decorators/wire'; import type { Program, diff --git a/packages/@lwc/ssr-compiler/src/compile-js/index.ts b/packages/@lwc/ssr-compiler/src/compile-js/index.ts index e6c8909a8d..9c075e9b23 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/index.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/index.ts @@ -9,27 +9,27 @@ import { generate } from 'astring'; import { traverse, builders as b, is } from 'estree-toolkit'; import { parseModule } from 'meriyah'; -import { DecoratorErrors } from '@lwc/errors'; import { transmogrify } from '../transmogrify'; import { ImportManager } from '../imports'; import { replaceLwcImport, replaceNamedLwcExport, replaceAllLwcExport } from './lwc-import'; import { catalogTmplImport } from './catalog-tmpls'; import { catalogStaticStylesheets, catalogAndReplaceStyleImports } from './stylesheets'; import { addGenerateMarkupFunction } from './generate-markup'; -import { catalogWireAdapters, isWireDecorator } from './wire'; -import { validateApiProperty, validateApiMethod } from './api/validate'; -import { isApiDecorator } from './api'; -import { isTrackDecorator } from './track'; +import { catalogWireAdapters, isWireDecorator } from './decorators/wire'; +import { validateApiProperty, validateApiMethod } from './decorators/api/validate'; +import { isApiDecorator } from './decorators/api'; import { removeDecoratorImport } from './remove-decorator-import'; -import { generateError } from './errors'; -import { type Visitors, type ComponentMetaState, isKeyIdentifier, isMethodKind } from './types'; +import { type Visitors, type ComponentMetaState } from './types'; +import { validateUniqueDecorator } from './decorators'; import type { ComponentTransformOptions } from '../shared'; import type { Identifier as EsIdentifier, Program as EsProgram, - Decorator as EsDecorator, + PropertyDefinition as EsPropertyDefinition, + MethodDefinition as EsMethodDefinition, + Identifier, } from 'estree'; import type { CompilationMode } from '@lwc/shared'; @@ -97,6 +97,7 @@ const visitors: Visitors = { }, PropertyDefinition(path, state) { const node = path.node; + if (!isKeyIdentifier(node)) { return; } @@ -144,7 +145,7 @@ const visitors: Visitors = { } else if (isWireDecorator(decorators[0])) { // Getters and setters are methods in the AST, but treated as properties by @wire // Note that this means that their implementations are ignored! - if (isMethodKind(node, ['get', 'set'])) { + if (node.kind === 'get' || node.kind === 'set') { const methodAsProp = b.propertyDefinition( structuredClone(node.key), null, @@ -206,30 +207,6 @@ const visitors: Visitors = { }, }; -function validateUniqueDecorator(decorators: EsDecorator[]) { - if (decorators.length < 2) { - return; - } - - const hasWire = decorators.some(isWireDecorator); - const hasApi = decorators.some(isApiDecorator); - const hasTrack = decorators.some(isTrackDecorator); - - if (hasWire) { - if (hasApi) { - throw generateError(DecoratorErrors.CONFLICT_WITH_ANOTHER_DECORATOR, 'api'); - } - - if (hasTrack) { - throw generateError(DecoratorErrors.CONFLICT_WITH_ANOTHER_DECORATOR, 'track'); - } - } - - if (hasApi && hasTrack) { - throw generateError(DecoratorErrors.API_AND_TRACK_DECORATOR_CONFLICT); - } -} - export default function compileJS( src: string, filename: string, @@ -282,3 +259,9 @@ export default function compileJS( code: generate(ast, {}), }; } + +function isKeyIdentifier( + node: T | undefined | null +): node is T & { key: Identifier } { + return node?.key.type === 'Identifier'; +} diff --git a/packages/@lwc/ssr-compiler/src/compile-js/types.ts b/packages/@lwc/ssr-compiler/src/compile-js/types.ts index 7d412eb819..98949a7764 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/types.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/types.ts @@ -25,19 +25,6 @@ export interface WireAdapter { field: MethodDefinition | PropertyDefinition; } -export function isMethodKind< - T extends MethodDefinition, - const K extends [T['kind'], ...T['kind'][]], ->(node: T, kind: K): node is T & { kind: K[number] } { - return kind.includes(node.kind); -} - -export function isKeyIdentifier( - node: T | undefined | null -): node is T & { key: Identifier } { - return node?.key.type === 'Identifier'; -} - export interface ComponentMetaState { // indicates whether the LightningElement subclass is found in the JS being traversed isLWC: boolean; @@ -62,7 +49,10 @@ export interface ComponentMetaState { // the set of variable names associated with explicitly imported CSS files staticStylesheetIds: Set | null; // the public (`@api`-annotated) fields of the component class - publicFields: Map; + publicFields: Map< + string, + (MethodDefinition & { key: Identifier }) | (PropertyDefinition & { key: Identifier }) + >; // the private fields of the component class privateFields: Set; // indicates whether the LightningElement has any wired props From 9ec59fb38641a389f550c63891734cccc887a30d Mon Sep 17 00:00:00 2001 From: Matheus Cardoso Date: Mon, 6 Jan 2025 11:51:53 -0300 Subject: [PATCH 11/11] chore: move duplicate consts to @lwc/shared --- .../babel-plugin-component/src/constants.ts | 21 ----------------- .../src/decorators/api/validate.ts | 8 ++----- packages/@lwc/shared/src/html-attributes.ts | 23 +++++++++++++++++++ .../src/compile-js/decorators/api/consts.ts | 15 ------------ .../src/compile-js/decorators/api/validate.ts | 2 +- 5 files changed, 26 insertions(+), 43 deletions(-) delete mode 100644 packages/@lwc/ssr-compiler/src/compile-js/decorators/api/consts.ts diff --git a/packages/@lwc/babel-plugin-component/src/constants.ts b/packages/@lwc/babel-plugin-component/src/constants.ts index f8be05db9f..0f4e07d703 100644 --- a/packages/@lwc/babel-plugin-component/src/constants.ts +++ b/packages/@lwc/babel-plugin-component/src/constants.ts @@ -5,25 +5,6 @@ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ -// This set is for attributes that have a camel cased property name -// For example, div.tabIndex. -// We do not want users to define @api properties with these names -// Because the template will never call them. It'll always call the camel -// cased version. -const AMBIGUOUS_PROP_SET = new Map([ - ['bgcolor', 'bgColor'], - ['accesskey', 'accessKey'], - ['contenteditable', 'contentEditable'], - ['tabindex', 'tabIndex'], - ['maxlength', 'maxLength'], - ['maxvalue', 'maxValue'], -]); - -// This set is for attributes that can never be defined -// by users on their components. -// We throw for these. -const DISALLOWED_PROP_SET = new Set(['is', 'class', 'slot', 'style']); - const LWC_PACKAGE_ALIAS = 'lwc'; const LWC_PACKAGE_EXPORTS = { @@ -55,9 +36,7 @@ const API_VERSION_KEY = 'apiVersion'; const COMPONENT_CLASS_ID = '__lwc_component_class_internal'; export { - AMBIGUOUS_PROP_SET, DECORATOR_TYPES, - DISALLOWED_PROP_SET, LWC_PACKAGE_ALIAS, LWC_PACKAGE_EXPORTS, LWC_COMPONENT_PROPERTIES, diff --git a/packages/@lwc/babel-plugin-component/src/decorators/api/validate.ts b/packages/@lwc/babel-plugin-component/src/decorators/api/validate.ts index 7ffdff4139..a021f05d6e 100644 --- a/packages/@lwc/babel-plugin-component/src/decorators/api/validate.ts +++ b/packages/@lwc/babel-plugin-component/src/decorators/api/validate.ts @@ -5,13 +5,9 @@ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ import { DecoratorErrors } from '@lwc/errors'; +import { AMBIGUOUS_PROP_SET, DISALLOWED_PROP_SET } from '@lwc/shared'; import { generateError } from '../../utils'; -import { - AMBIGUOUS_PROP_SET, - DECORATOR_TYPES, - DISALLOWED_PROP_SET, - LWC_PACKAGE_EXPORTS, -} from '../../constants'; +import { DECORATOR_TYPES, LWC_PACKAGE_EXPORTS } from '../../constants'; import { isApiDecorator } from './shared'; import type { types, NodePath } from '@babel/core'; import type { LwcBabelPluginPass } from '../../types'; diff --git a/packages/@lwc/shared/src/html-attributes.ts b/packages/@lwc/shared/src/html-attributes.ts index ac37b419fc..c35367767d 100644 --- a/packages/@lwc/shared/src/html-attributes.ts +++ b/packages/@lwc/shared/src/html-attributes.ts @@ -194,3 +194,26 @@ export function kebabCaseToCamelCase(attrName: string): string { return result; } + +/** + * This set is for attributes that have a camel cased property name + * For example, div.tabIndex. + * We do not want users to define `@api` properties with these names + * Because the template will never call them. It'll always call the camel + * cased version. + */ +export const AMBIGUOUS_PROP_SET = /*@__PURE__@*/ new Map([ + ['bgcolor', 'bgColor'], + ['accesskey', 'accessKey'], + ['contenteditable', 'contentEditable'], + ['tabindex', 'tabIndex'], + ['maxlength', 'maxLength'], + ['maxvalue', 'maxValue'], +]); + +/** + * This set is for attributes that can never be defined + * by users on their components. + * We throw for these. + */ +export const DISALLOWED_PROP_SET = /*@__PURE__@*/ new Set(['is', 'class', 'slot', 'style']); diff --git a/packages/@lwc/ssr-compiler/src/compile-js/decorators/api/consts.ts b/packages/@lwc/ssr-compiler/src/compile-js/decorators/api/consts.ts deleted file mode 100644 index bfa76f9a18..0000000000 --- a/packages/@lwc/ssr-compiler/src/compile-js/decorators/api/consts.ts +++ /dev/null @@ -1,15 +0,0 @@ -/* - * Copyright (c) 2024, Salesforce, Inc. - * All rights reserved. - * SPDX-License-Identifier: MIT - * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT - */ -export const DISALLOWED_PROP_SET = new Set(['is', 'class', 'slot', 'style']); -export const AMBIGUOUS_PROP_SET = new Map([ - ['bgcolor', 'bgColor'], - ['accesskey', 'accessKey'], - ['contenteditable', 'contentEditable'], - ['tabindex', 'tabIndex'], - ['maxlength', 'maxLength'], - ['maxvalue', 'maxValue'], -]); diff --git a/packages/@lwc/ssr-compiler/src/compile-js/decorators/api/validate.ts b/packages/@lwc/ssr-compiler/src/compile-js/decorators/api/validate.ts index f0f4badee3..e911abe9a4 100644 --- a/packages/@lwc/ssr-compiler/src/compile-js/decorators/api/validate.ts +++ b/packages/@lwc/ssr-compiler/src/compile-js/decorators/api/validate.ts @@ -6,9 +6,9 @@ */ import { DecoratorErrors } from '@lwc/errors'; +import { DISALLOWED_PROP_SET, AMBIGUOUS_PROP_SET } from '@lwc/shared'; import { generateError } from '../../errors'; import { type ComponentMetaState } from '../../types'; -import { DISALLOWED_PROP_SET, AMBIGUOUS_PROP_SET } from './consts'; import type { Identifier, MethodDefinition, PropertyDefinition } from 'estree'; export type ApiMethodDefinition = MethodDefinition & {