Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ssr-compiler): validate api decorator usage #5071

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions packages/@lwc/babel-plugin-component/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
23 changes: 23 additions & 0 deletions packages/@lwc/shared/src/html-attributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
Comment on lines +197 to +219
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe @lwc/errors would be a better place for this. Not sure.

168 changes: 168 additions & 0 deletions packages/@lwc/ssr-compiler/src/__tests__/api-decorator.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
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', () => {
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`);
});
cardoso marked this conversation as resolved.
Show resolved Hide resolved

describe('conflicting api properties', () => {
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";
}`,
],
])(`%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', () => {
const src = /* js */ `
import { api, LightningElement } from "lwc";
export default class Test extends LightningElement {
@api publicProp = true;
}
`;
expect(compile(src)).toThrow(`LWC1099: Boolean public property must default to false.`);
});

test('computed api getters and setters', () => {
const src = /* js */ `
import { api, LightningElement } from "lwc";
export default class Test extends LightningElement {
@api
set [x](value) {}
get [x]() {}
}
`;
expect(compile(src)).toThrow(
`LWC1106: @api cannot be applied to a computed property, getter, setter or method.`
);
});

test('property name prefixed with data', () => {
const src = /* js */ `
import { api, LightningElement } from "lwc";
export default class Test extends LightningElement {
@api dataFooBar;
}
`;
expect(compile(src)).toThrow(
`LWC1107: Invalid property name "dataFooBar". Properties starting with "data" are reserved attributes.`
);
});

test('property name prefixed with on', () => {
const src = /* js */ `
import { api, LightningElement } from "lwc";
export default class Test extends LightningElement {
@api onChangeHandler;
}
`;
expect(compile(src)).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 [prop: string, suggestion: string][])('%s', ([prop, suggestion]) => {
const src = /* js */ `
import { api, LightningElement } from "lwc";
export default class Test extends LightningElement {
@api ${prop};
}
`;
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}"`
);
});
});

describe('disallowed props', () => {
test.for(['class', 'is', 'slot', 'style'])('%s', (prop) => {
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', () => {
const src = /* js */ `
import { api, LightningElement } from "lwc";
export default class Test extends LightningElement {
@api part;
}
`;
expect(compile(src)).toThrow(
`LWC1111: Invalid property name "part". "part" is a future reserved attribute for web components.`
);
});

test('both getter and a setter', () => {
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;
}
}
`;
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.`
);
});
});
Original file line number Diff line number Diff line change
@@ -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
*/
import type { Decorator, Identifier } from 'estree';

export function isApiDecorator(decorator: Decorator | undefined): decorator is Decorator & {
expression: Identifier & {
name: 'api';
};
} {
return decorator?.expression.type === 'Identifier' && decorator.expression.name === 'api';
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* 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 { DISALLOWED_PROP_SET, AMBIGUOUS_PROP_SET } from '@lwc/shared';
import { generateError } from '../../errors';
import { type ComponentMetaState } 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) {
throw generateError(DecoratorErrors.PROPERTY_CANNOT_BE_COMPUTED);
}

const propertyName = definition.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)!
);
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole file mirrors the structure and logic from babel-plugin-component:

function validatePropertyName(property: NodePath<types.ClassMethod>, state: LwcBabelPluginPass) {
if (property.node.computed) {
throw generateError(
property,
{
errorInfo: DecoratorErrors.PROPERTY_CANNOT_BE_COMPUTED,
},
state
);
}


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 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) {
return;
}

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) {
validateUniqueMethod(node, state);
validateName(node);
}
36 changes: 36 additions & 0 deletions packages/@lwc/ssr-compiler/src/compile-js/decorators/index.ts
Original file line number Diff line number Diff line change
@@ -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);
}
}
Loading