Skip to content

Commit 62fecca

Browse files
cardosowjhsf
cardoso
and
wjhsf
authored
feat(ssr-compiler): validate api decorator usage (#5071)
* feat(ssr): validate api decorator * chore: simplify test * feat: validate all api decorator errors * chore: move api-decorator tests to its own file * chore: format test * chore: separate api decorator validation into its own file * chore: improve validation functions * chore: encapsulate api validators * chore: create api folder * chore: move decorators to their own folder * chore: move duplicate consts to @lwc/shared * chore: remove commented code * chore: simplify publicField types Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com> * chore: use is.identifier Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com> * chore: use is.literal Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com> * fix: type errors --------- Co-authored-by: Will Harney <62956339+wjhsf@users.noreply.github.com>
1 parent 76b9d1d commit 62fecca

File tree

13 files changed

+419
-106
lines changed

13 files changed

+419
-106
lines changed

‎packages/@lwc/babel-plugin-component/src/constants.ts‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,25 +5,6 @@
55
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
66
*/
77

8-
// This set is for attributes that have a camel cased property name
9-
// For example, div.tabIndex.
10-
// We do not want users to define @api properties with these names
11-
// Because the template will never call them. It'll always call the camel
12-
// cased version.
13-
const AMBIGUOUS_PROP_SET = new Map([
14-
['bgcolor', 'bgColor'],
15-
['accesskey', 'accessKey'],
16-
['contenteditable', 'contentEditable'],
17-
['tabindex', 'tabIndex'],
18-
['maxlength', 'maxLength'],
19-
['maxvalue', 'maxValue'],
20-
]);
21-
22-
// This set is for attributes that can never be defined
23-
// by users on their components.
24-
// We throw for these.
25-
const DISALLOWED_PROP_SET = new Set(['is', 'class', 'slot', 'style']);
26-
278
const LWC_PACKAGE_ALIAS = 'lwc';
289

2910
const LWC_PACKAGE_EXPORTS = {
@@ -55,9 +36,7 @@ const API_VERSION_KEY = 'apiVersion';
5536
const COMPONENT_CLASS_ID = '__lwc_component_class_internal';
5637

5738
export {
58-
AMBIGUOUS_PROP_SET,
5939
DECORATOR_TYPES,
60-
DISALLOWED_PROP_SET,
6140
LWC_PACKAGE_ALIAS,
6241
LWC_PACKAGE_EXPORTS,
6342
LWC_COMPONENT_PROPERTIES,

‎packages/@lwc/babel-plugin-component/src/decorators/api/validate.ts‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,9 @@
55
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
66
*/
77
import { DecoratorErrors } from '@lwc/errors';
8+
import { AMBIGUOUS_PROP_SET, DISALLOWED_PROP_SET } from '@lwc/shared';
89
import { generateError } from '../../utils';
9-
import {
10-
AMBIGUOUS_PROP_SET,
11-
DECORATOR_TYPES,
12-
DISALLOWED_PROP_SET,
13-
LWC_PACKAGE_EXPORTS,
14-
} from '../../constants';
10+
import { DECORATOR_TYPES, LWC_PACKAGE_EXPORTS } from '../../constants';
1511
import { isApiDecorator } from './shared';
1612
import type { types, NodePath } from '@babel/core';
1713
import type { LwcBabelPluginPass } from '../../types';

‎packages/@lwc/shared/src/html-attributes.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,3 +194,26 @@ export function kebabCaseToCamelCase(attrName: string): string {
194194

195195
return result;
196196
}
197+
198+
/**
199+
* This set is for attributes that have a camel cased property name
200+
* For example, div.tabIndex.
201+
* We do not want users to define `@api` properties with these names
202+
* Because the template will never call them. It'll always call the camel
203+
* cased version.
204+
*/
205+
export const AMBIGUOUS_PROP_SET = /*@__PURE__@*/ new Map([
206+
['bgcolor', 'bgColor'],
207+
['accesskey', 'accessKey'],
208+
['contenteditable', 'contentEditable'],
209+
['tabindex', 'tabIndex'],
210+
['maxlength', 'maxLength'],
211+
['maxvalue', 'maxValue'],
212+
]);
213+
214+
/**
215+
* This set is for attributes that can never be defined
216+
* by users on their components.
217+
* We throw for these.
218+
*/
219+
export const DISALLOWED_PROP_SET = /*@__PURE__@*/ new Set(['is', 'class', 'slot', 'style']);

‎packages/@lwc/ssr-compiler/src/__tests__/api-decorator.spec.ts‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
import { describe, test, expect } from 'vitest';
2+
import { compileComponentForSSR } from '../index';
3+
4+
const compile =
5+
(src: string, filename = 'test.js') =>
6+
() => {
7+
return compileComponentForSSR(src, filename, {});
8+
};
9+
10+
describe('thows error', () => {
11+
test('combined with @track', () => {
12+
const src = /* js */ `
13+
import { api, track, LightningElement } from "lwc";
14+
export default class Test extends LightningElement {
15+
@track
16+
@api
17+
apiWithTrack = "foo";
18+
}
19+
`;
20+
expect(compile(src)).toThrow(`LWC1093: @api method or property cannot be used with @track`);
21+
});
22+
23+
describe('conflicting api properties', () => {
24+
test.for([
25+
[
26+
'getter/setter',
27+
/* js */ `
28+
@api foo = 1;
29+
_internal = 1;
30+
@api
31+
get foo() {
32+
return "foo";
33+
}
34+
set foo(val) {
35+
this._internal = val;
36+
}`,
37+
],
38+
[
39+
'method',
40+
/* js */ `
41+
@api foo = 1;
42+
@api foo() {
43+
return "foo";
44+
}`,
45+
],
46+
])(`%s`, ([, body]) => {
47+
const src = /* js */ `
48+
import { api, LightningElement } from "lwc";
49+
export default class Test extends LightningElement {
50+
${body}
51+
}
52+
`;
53+
expect(compile(src)).toThrow(`LWC1096: Duplicate @api property "foo".`);
54+
});
55+
});
56+
57+
test('default value is true', () => {
58+
const src = /* js */ `
59+
import { api, LightningElement } from "lwc";
60+
export default class Test extends LightningElement {
61+
@api publicProp = true;
62+
}
63+
`;
64+
expect(compile(src)).toThrow(`LWC1099: Boolean public property must default to false.`);
65+
});
66+
67+
test('computed api getters and setters', () => {
68+
const src = /* js */ `
69+
import { api, LightningElement } from "lwc";
70+
export default class Test extends LightningElement {
71+
@api
72+
set [x](value) {}
73+
get [x]() {}
74+
}
75+
`;
76+
expect(compile(src)).toThrow(
77+
`LWC1106: @api cannot be applied to a computed property, getter, setter or method.`
78+
);
79+
});
80+
81+
test('property name prefixed with data', () => {
82+
const src = /* js */ `
83+
import { api, LightningElement } from "lwc";
84+
export default class Test extends LightningElement {
85+
@api dataFooBar;
86+
}
87+
`;
88+
expect(compile(src)).toThrow(
89+
`LWC1107: Invalid property name "dataFooBar". Properties starting with "data" are reserved attributes.`
90+
);
91+
});
92+
93+
test('property name prefixed with on', () => {
94+
const src = /* js */ `
95+
import { api, LightningElement } from "lwc";
96+
export default class Test extends LightningElement {
97+
@api onChangeHandler;
98+
}
99+
`;
100+
expect(compile(src)).toThrow(
101+
`LWC1108: Invalid property name "onChangeHandler". Properties starting with "on" are reserved for event handlers`
102+
);
103+
});
104+
105+
describe('property name is ambiguous', () => {
106+
test.for([
107+
['bgcolor', 'bgColor'],
108+
['accesskey', 'accessKey'],
109+
['contenteditable', 'contentEditable'],
110+
['tabindex', 'tabIndex'],
111+
['maxlength', 'maxLength'],
112+
['maxvalue', 'maxValue'],
113+
] as [prop: string, suggestion: string][])('%s', ([prop, suggestion]) => {
114+
const src = /* js */ `
115+
import { api, LightningElement } from "lwc";
116+
export default class Test extends LightningElement {
117+
@api ${prop};
118+
}
119+
`;
120+
expect(compile(src)).toThrow(
121+
`LWC1109: Ambiguous attribute name "${prop}". "${prop}" will never be called from template because its corresponding property is camel cased. Consider renaming to "${suggestion}"`
122+
);
123+
});
124+
});
125+
126+
describe('disallowed props', () => {
127+
test.for(['class', 'is', 'slot', 'style'])('%s', (prop) => {
128+
const src = /* js */ `
129+
import { api, LightningElement } from 'lwc'
130+
export default class Test extends LightningElement {
131+
@api ${prop}
132+
}
133+
`;
134+
expect(compile(src)).toThrow(
135+
`LWC1110: Invalid property name "${prop}". "${prop}" is a reserved attribute.`
136+
);
137+
});
138+
});
139+
140+
test('property name is part', () => {
141+
const src = /* js */ `
142+
import { api, LightningElement } from "lwc";
143+
export default class Test extends LightningElement {
144+
@api part;
145+
}
146+
`;
147+
expect(compile(src)).toThrow(
148+
`LWC1111: Invalid property name "part". "part" is a future reserved attribute for web components.`
149+
);
150+
});
151+
152+
test('both getter and a setter', () => {
153+
const src = /* js */ `
154+
import { api, LightningElement } from "lwc";
155+
export default class Test extends LightningElement {
156+
@api get something() {
157+
return this.s;
158+
}
159+
@api set something(value) {
160+
this.s = value;
161+
}
162+
}
163+
`;
164+
expect(compile(src)).toThrow(
165+
`LWC1112: @api get something and @api set something detected in class declaration. Only one of the two needs to be decorated with @api.`
166+
);
167+
});
168+
});

‎packages/@lwc/ssr-compiler/src/compile-js/decorators/api/index.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/*
2+
* Copyright (c) 2024, Salesforce, Inc.
3+
* All rights reserved.
4+
* SPDX-License-Identifier: MIT
5+
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
6+
*/
7+
import type { Decorator, Identifier } from 'estree';
8+
9+
export function isApiDecorator(decorator: Decorator | undefined): decorator is Decorator & {
10+
expression: Identifier & {
11+
name: 'api';
12+
};
13+
} {
14+
return decorator?.expression.type === 'Identifier' && decorator.expression.name === 'api';
15+
}

‎packages/@lwc/ssr-compiler/src/compile-js/decorators/api/validate.ts‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/*
2+
* Copyright (c) 2024, Salesforce, Inc.
3+
* All rights reserved.
4+
* SPDX-License-Identifier: MIT
5+
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
6+
*/
7+
8+
import { DecoratorErrors } from '@lwc/errors';
9+
import { DISALLOWED_PROP_SET, AMBIGUOUS_PROP_SET } from '@lwc/shared';
10+
import { is } from 'estree-toolkit';
11+
import { generateError } from '../../errors';
12+
import { type ComponentMetaState } from '../../types';
13+
import type { Identifier, MethodDefinition, PropertyDefinition } from 'estree';
14+
export type ApiMethodDefinition = MethodDefinition & {
15+
key: Identifier;
16+
};
17+
export type ApiPropertyDefinition = PropertyDefinition & {
18+
key: Identifier;
19+
};
20+
21+
export type ApiDefinition = ApiPropertyDefinition | ApiMethodDefinition;
22+
23+
function validateName(definition: ApiDefinition) {
24+
if (definition.computed) {
25+
throw generateError(definition, DecoratorErrors.PROPERTY_CANNOT_BE_COMPUTED);
26+
}
27+
28+
const propertyName = definition.key.name;
29+
30+
switch (true) {
31+
case propertyName === 'part':
32+
throw generateError(
33+
definition,
34+
DecoratorErrors.PROPERTY_NAME_PART_IS_RESERVED,
35+
propertyName
36+
);
37+
case propertyName.startsWith('on'):
38+
throw generateError(
39+
definition,
40+
DecoratorErrors.PROPERTY_NAME_CANNOT_START_WITH_ON,
41+
propertyName
42+
);
43+
case propertyName.startsWith('data') && propertyName.length > 4:
44+
throw generateError(
45+
definition,
46+
DecoratorErrors.PROPERTY_NAME_CANNOT_START_WITH_DATA,
47+
propertyName
48+
);
49+
case DISALLOWED_PROP_SET.has(propertyName):
50+
throw generateError(
51+
definition,
52+
DecoratorErrors.PROPERTY_NAME_IS_RESERVED,
53+
propertyName
54+
);
55+
case AMBIGUOUS_PROP_SET.has(propertyName):
56+
throw generateError(
57+
definition,
58+
DecoratorErrors.PROPERTY_NAME_IS_AMBIGUOUS,
59+
propertyName,
60+
AMBIGUOUS_PROP_SET.get(propertyName)!
61+
);
62+
}
63+
}
64+
65+
function validatePropertyValue(property: ApiPropertyDefinition) {
66+
if (is.literal(property.value) && property.value.value === true) {
67+
throw generateError(property, DecoratorErrors.INVALID_BOOLEAN_PUBLIC_PROPERTY);
68+
}
69+
}
70+
71+
function validatePropertyUnique(node: ApiPropertyDefinition, state: ComponentMetaState) {
72+
if (state.publicProperties.has(node.key.name)) {
73+
throw generateError(node, DecoratorErrors.DUPLICATE_API_PROPERTY, node.key.name);
74+
}
75+
}
76+
77+
export function validateApiProperty(node: ApiPropertyDefinition, state: ComponentMetaState) {
78+
validatePropertyUnique(node, state);
79+
validateName(node);
80+
validatePropertyValue(node);
81+
}
82+
83+
function validateUniqueMethod(node: ApiMethodDefinition, state: ComponentMetaState) {
84+
const field = state.publicProperties.get(node.key.name);
85+
86+
if (!field) {
87+
return;
88+
}
89+
90+
if (
91+
field.type === 'MethodDefinition' &&
92+
(field.kind === 'get' || field.kind === 'set') &&
93+
(node.kind === 'get' || node.kind === 'set')
94+
) {
95+
throw generateError(
96+
node,
97+
DecoratorErrors.SINGLE_DECORATOR_ON_SETTER_GETTER_PAIR,
98+
node.key.name
99+
);
100+
}
101+
102+
throw generateError(node, DecoratorErrors.DUPLICATE_API_PROPERTY, node.key.name);
103+
}
104+
105+
export function validateApiMethod(node: ApiMethodDefinition, state: ComponentMetaState) {
106+
validateUniqueMethod(node, state);
107+
validateName(node);
108+
}

0 commit comments

Comments
 (0)