Skip to content

Commit fd29cd6

Browse files
seckardtseckardt
authored
feat: update rule no-unsupported-ssr-properties to reflect SSR best practices @W-14387292 (#133)
* feat: update rule `no-unsupported-ssr-properties` to reflect best practices @W-14387292 * feat: enforce fully optional expression chains @W-14387292
1 parent 47efeac commit fd29cd6

7 files changed

+288
-18
lines changed

‎docs/rules/no-restricted-browser-globals-during-ssr.md‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,16 +65,6 @@ export default class Foo extends LightningElement {
6565
```js
6666
import { LightningElement } from 'lwc';
6767

68-
export default class Foo extends LightningElement {
69-
renderedCallback() {
70-
const parser = new DOMParser();
71-
}
72-
}
73-
```
74-
75-
```js
76-
import { LightningElement } from 'lwc';
77-
7868
export default class Foo extends LightningElement {
7969
constructor() {
8070
this.handleResize = this.handleResize.bind(this);
@@ -101,6 +91,16 @@ export default class Foo extends LightningElement {
10191
}
10292
```
10393
94+
```js
95+
import { LightningElement } from 'lwc';
96+
97+
export default class Foo extends LightningElement {
98+
renderedCallback() {
99+
const parser = new DOMParser();
100+
}
101+
}
102+
```
103+
104104
## Options
105105
106106
The rule takes one option, an object, which has one key `restricted-globals` which is an object. The keys in the object

‎docs/rules/no-unsupported-ssr-properties.md‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,42 @@ Examples of **correct** code for this rule:
2929
```js
3030
import { LightningElement } from 'lwc';
3131

32+
export default class Foo extends LightningElement {
33+
connectedCallback() {
34+
this.querySelector?.('span')?.getAttribute?.('role');
35+
}
36+
}
37+
38+
export default class Foo extends LightningElement {
39+
connectedCallback() {
40+
this.dispatchEvent?.(new CustomEvent('customevent'));
41+
}
42+
}
43+
```
44+
45+
```js
46+
import { LightningElement } from 'lwc';
47+
48+
export default class Foo extends LightningElement {
49+
connectedCallback() {
50+
if (!import.meta.env.SSR) {
51+
this.querySelector('span')?.getAttribute('role');
52+
}
53+
}
54+
}
55+
56+
export default class Foo extends LightningElement {
57+
connectedCallback() {
58+
if (!import.meta.env.SSR) {
59+
this.dispatchEvent(new CustomEvent('customevent'));
60+
}
61+
}
62+
}
63+
```
64+
65+
```js
66+
import { LightningElement } from 'lwc';
67+
3268
export default class Foo extends LightningElement {
3369
renderedCallback() {
3470
this.querySelector('span')?.foo();
@@ -37,6 +73,8 @@ export default class Foo extends LightningElement {
3773

3874
export default class Foo extends LightningElement {
3975
renderedCallback() {
76+
// Caution: This lifecycle hook is very likely
77+
// to be called more than once.
4078
this.dispatchEvent(new CustomEvent('customevent'));
4179
}
4280
}

‎lib/rule-helpers.js‎

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ const { isSSREscape } = require('./util/ssr');
1010
const { isGlobalIdentifier } = require('./util/scope');
1111

1212
/**
13-
* Visitors for detecting methods/functions that are reacheable during SSR
14-
* @param {import('eslint').Rule.RuleContext} context
13+
* Visitors for detecting methods/functions that are reachable during SSR
1514
*/
1615
function reachableDuringSSRPartial() {
1716
let moduleInfo;
@@ -117,6 +116,14 @@ const moduleScopeDisqualifiers = new Set([
117116
'ArrowFunctionExpression',
118117
]);
119118

119+
const noReferenceParentQualifiers = new Set([
120+
'CallExpression',
121+
'ExpressionStatement',
122+
'AssignmentExpression',
123+
]);
124+
125+
const globalAccessQualifiers = new Set(['CallExpression', 'MemberExpression']);
126+
120127
function inModuleScope(node, context) {
121128
for (const ancestor of context.getAncestors()) {
122129
if (moduleScopeDisqualifiers.has(ancestor.type)) {
@@ -149,13 +156,14 @@ module.exports.noReferenceDuringSSR = function noReferenceDuringSSR(
149156
) {
150157
return;
151158
}
159+
152160
if (
153161
node.parent.type === 'MemberExpression' &&
162+
node.parent.optional !== true &&
154163
node.object.type === 'Identifier' &&
155164
node.object.name === 'globalThis' &&
156165
node.property.type === 'Identifier' &&
157166
forbiddenGlobalNames.has(node.property.name) &&
158-
node.parent.optional !== true &&
159167
isGlobalIdentifier(node.object, context.getScope())
160168
) {
161169
// Prevents expressions like:
@@ -207,7 +215,7 @@ module.exports.noReferenceDuringSSR = function noReferenceDuringSSR(
207215
return;
208216
}
209217
if (
210-
node.parent.type !== 'MemberExpression' &&
218+
noReferenceParentQualifiers.has(node.parent.type) &&
211219
forbiddenGlobalNames.has(node.name) &&
212220
isGlobalIdentifier(node, context.getScope())
213221
) {
@@ -229,25 +237,82 @@ module.exports.noReferenceDuringSSR = function noReferenceDuringSSR(
229237
};
230238
};
231239

240+
/**
241+
* Reports issues about accessing unsupported LWC class methods in SSR.
242+
* @see {@link https://github.com/salesforce/lwc/blob/master/packages/%40lwc/engine-server/src/renderer.ts}
243+
*/
232244
module.exports.noPropertyAccessDuringSSR = function noPropertyAccessDuringSSR(
233245
forbiddenPropertyNames,
234246
reporter,
235247
) {
236248
const { withinLWCVisitors, isInsideReachableMethod, isInsideSkippedBlock } =
237249
reachableDuringSSRPartial();
250+
let expressionStatementWeAreIn = null;
251+
let memberExpressionsInStatement = [];
252+
let callExpressionsInStatement = [];
238253

239254
return {
240255
...withinLWCVisitors,
256+
ExpressionStatement: (node) => {
257+
expressionStatementWeAreIn = node;
258+
callExpressionsInStatement = [];
259+
memberExpressionsInStatement = [];
260+
},
261+
'ExpressionStatement:exit': (node) => {
262+
if (expressionStatementWeAreIn === node) {
263+
expressionStatementWeAreIn = null;
264+
callExpressionsInStatement = [];
265+
memberExpressionsInStatement = [];
266+
}
267+
},
268+
AssignmentExpression: (node) => {
269+
callExpressionsInStatement.push(node);
270+
},
271+
CallExpression: (node) => {
272+
callExpressionsInStatement.push(node);
273+
},
241274
MemberExpression: (node) => {
242275
if (!isInsideReachableMethod() || isInsideSkippedBlock()) {
243276
return;
244277
}
278+
279+
memberExpressionsInStatement.push(node);
280+
245281
if (
246282
node.object.type === 'ThisExpression' &&
283+
globalAccessQualifiers.has(node.parent.type) &&
247284
node.property.type === 'Identifier' &&
248285
forbiddenPropertyNames.includes(node.property.name)
249286
) {
250-
reporter(node);
287+
// Prevents expressions like:
288+
// this.dispatchEvent(new CustomEvent('myevent'));
289+
// this.querySelector('button').addEventListener('click', ...);
290+
// this.querySelector?.('button').addEventListener('click', ...);
291+
// this.querySelector?.('button')?.addEventListener('click', ...);
292+
// this.querySelector?.('button').firstElementChild.id;
293+
// this.childNodes.item(0).textContent = 'foo';
294+
295+
// Allows all-optional expressions like:
296+
// this.dispatchEvent?.(new CustomEvent('myevent'));
297+
// this.querySelector?.('button')?.addEventListener?.('click', ...);
298+
// this.querySelector?.('button')?.firstElementChild.id;
299+
const allCallExpressionsOptional = callExpressionsInStatement.every(
300+
(expression) => expression.optional,
301+
);
302+
const allMemberExpressionsOptional = memberExpressionsInStatement.every(
303+
(expression, index) => {
304+
if (expression.parent && expression.parent.type === 'CallExpression') {
305+
// Skip CallExpressions here as they are treated separately
306+
return true;
307+
}
308+
// Return `true` if the MemberExpression is either `optional` or
309+
// the last expression of the chain (which is in revered order).
310+
return expression.optional || index === 0;
311+
},
312+
);
313+
if (!allCallExpressionsOptional || !allMemberExpressionsOptional) {
314+
reporter(node);
315+
}
251316
}
252317
},
253318
};

‎lib/rules/no-unsupported-ssr-properties.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ const { noPropertyAccessDuringSSR } = require('../rule-helpers');
99
const { docUrl } = require('../util/doc-url');
1010

1111
const disallowedProperties = [
12+
'attachInternals',
13+
'children',
14+
'childNodes',
1215
'dispatchEvent',
1316
'firstChild',
1417
'firstElementChild',
@@ -20,7 +23,6 @@ const disallowedProperties = [
2023
'ownerDocument',
2124
'querySelector',
2225
'querySelectorAll',
23-
'removeEventListener',
2426
];
2527
module.exports = {
2628
meta: {
@@ -32,7 +34,8 @@ module.exports = {
3234
},
3335
schema: [],
3436
messages: {
35-
propertyAccessFound: '`{{ identifier }}` is unsupported in SSR.',
37+
propertyAccessFound:
38+
'`{{ identifier }}` is unsupported in SSR. Consider guarding access to `{{identifier}}`, e.g. via the `import.meta.env.SSR` flag, or optional chaining (`this.{{identifier}}?.`).',
3639
},
3740
},
3841
create: (context) => {

‎test/lib/rules/consistent-component-name.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ const rule = require('../../../lib/rules/consistent-component-name');
1313

1414
const ruleTester = new RuleTester(ESLINT_TEST_CONFIG);
1515

16-
ruleTester.run('consistent-class-name', rule, {
16+
ruleTester.run('consistent-component-name', rule, {
1717
valid: [
1818
{
1919
code: `

‎test/lib/rules/no-restricted-browser-globals-during-ssr.js‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,18 @@ tester.run('no-browser-globals-during-ssr', rule, {
192192
}
193193
`,
194194
},
195+
{
196+
code: `
197+
let name;
198+
let screen = 'mobile';
199+
export default class Foo extends LightningElement {
200+
connectedCallback() {
201+
name = "hello";
202+
screen;
203+
}
204+
}
205+
`,
206+
},
195207
{
196208
code: `
197209
import { name } from 'some/component';
@@ -639,5 +651,22 @@ tester.run('no-browser-globals-during-ssr', rule, {
639651
},
640652
],
641653
},
654+
{
655+
code: `
656+
import { LightningElement } from 'lwc';
657+
658+
export default class Foo extends LightningElement {
659+
connectedCallback() {
660+
name = 'hello';
661+
}
662+
}
663+
`,
664+
errors: [
665+
{
666+
messageId: 'prohibitedBrowserAPIUsage',
667+
data: { identifier: 'name' },
668+
},
669+
],
670+
},
642671
],
643672
});

0 commit comments

Comments
 (0)