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

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

@@ -65,16 +65,6 @@ export default class Foo extends LightningElement {
 ```js
 import { LightningElement } from 'lwc';
 
-export default class Foo extends LightningElement {
-    renderedCallback() {
-        const parser = new DOMParser();
-    }
-}
-```
-
-```js
-import { LightningElement } from 'lwc';
-
 export default class Foo extends LightningElement {
     constructor() {
         this.handleResize = this.handleResize.bind(this);
@@ -101,6 +91,16 @@ export default class Foo extends LightningElement {
 }
 ```
 
+```js
+import { LightningElement } from 'lwc';
+
+export default class Foo extends LightningElement {
+    renderedCallback() {
+        const parser = new DOMParser();
+    }
+}
+```
+
 ## Options
 
 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

@@ -29,6 +29,42 @@ Examples of **correct** code for this rule:
 ```js
 import { LightningElement } from 'lwc';
 
+export default class Foo extends LightningElement {
+    connectedCallback() {
+        this.querySelector?.('span')?.getAttribute?.('role');
+    }
+}
+
+export default class Foo extends LightningElement {
+    connectedCallback() {
+        this.dispatchEvent?.(new CustomEvent('customevent'));
+    }
+}
+```
+
+```js
+import { LightningElement } from 'lwc';
+
+export default class Foo extends LightningElement {
+    connectedCallback() {
+        if (!import.meta.env.SSR) {
+            this.querySelector('span')?.getAttribute('role');
+        }
+    }
+}
+
+export default class Foo extends LightningElement {
+    connectedCallback() {
+        if (!import.meta.env.SSR) {
+            this.dispatchEvent(new CustomEvent('customevent'));
+        }
+    }
+}
+```
+
+```js
+import { LightningElement } from 'lwc';
+
 export default class Foo extends LightningElement {
     renderedCallback() {
         this.querySelector('span')?.foo();
@@ -37,6 +73,8 @@ export default class Foo extends LightningElement {
 
 export default class Foo extends LightningElement {
     renderedCallback() {
+        // Caution: This lifecycle hook is very likely
+        // to be called more than once.
         this.dispatchEvent(new CustomEvent('customevent'));
     }
 }

lib/rule-helpers.js

@@ -10,8 +10,7 @@ const { isSSREscape } = require('./util/ssr');
 const { isGlobalIdentifier } = require('./util/scope');
 
 /**
- * Visitors for detecting methods/functions that are reacheable during SSR
- * @param {import('eslint').Rule.RuleContext} context
+ * Visitors for detecting methods/functions that are reachable during SSR
  */
 function reachableDuringSSRPartial() {
     let moduleInfo;
@@ -117,6 +116,14 @@ const moduleScopeDisqualifiers = new Set([
     'ArrowFunctionExpression',
 ]);
 
+const noReferenceParentQualifiers = new Set([
+    'CallExpression',
+    'ExpressionStatement',
+    'AssignmentExpression',
+]);
+
+const globalAccessQualifiers = new Set(['CallExpression', 'MemberExpression']);
+
 function inModuleScope(node, context) {
     for (const ancestor of context.getAncestors()) {
         if (moduleScopeDisqualifiers.has(ancestor.type)) {
@@ -149,13 +156,14 @@ module.exports.noReferenceDuringSSR = function noReferenceDuringSSR(
             ) {
                 return;
             }
+
             if (
                 node.parent.type === 'MemberExpression' &&
+                node.parent.optional !== true &&
                 node.object.type === 'Identifier' &&
                 node.object.name === 'globalThis' &&
                 node.property.type === 'Identifier' &&
                 forbiddenGlobalNames.has(node.property.name) &&
-                node.parent.optional !== true &&
                 isGlobalIdentifier(node.object, context.getScope())
             ) {
                 // Prevents expressions like:
@@ -207,7 +215,7 @@ module.exports.noReferenceDuringSSR = function noReferenceDuringSSR(
                 return;
             }
             if (
-                node.parent.type !== 'MemberExpression' &&
+                noReferenceParentQualifiers.has(node.parent.type) &&
                 forbiddenGlobalNames.has(node.name) &&
                 isGlobalIdentifier(node, context.getScope())
             ) {
@@ -229,25 +237,82 @@ module.exports.noReferenceDuringSSR = function noReferenceDuringSSR(
     };
 };
 
+/**
+ * Reports issues about accessing unsupported LWC class methods in SSR.
+ * @see {@link https://github.com/salesforce/lwc/blob/master/packages/%40lwc/engine-server/src/renderer.ts}
+ */
 module.exports.noPropertyAccessDuringSSR = function noPropertyAccessDuringSSR(
     forbiddenPropertyNames,
     reporter,
 ) {
     const { withinLWCVisitors, isInsideReachableMethod, isInsideSkippedBlock } =
         reachableDuringSSRPartial();
+    let expressionStatementWeAreIn = null;
+    let memberExpressionsInStatement = [];
+    let callExpressionsInStatement = [];
 
     return {
         ...withinLWCVisitors,
+        ExpressionStatement: (node) => {
+            expressionStatementWeAreIn = node;
+            callExpressionsInStatement = [];
+            memberExpressionsInStatement = [];
+        },
+        'ExpressionStatement:exit': (node) => {
+            if (expressionStatementWeAreIn === node) {
+                expressionStatementWeAreIn = null;
+                callExpressionsInStatement = [];
+                memberExpressionsInStatement = [];
+            }
+        },
+        AssignmentExpression: (node) => {
+            callExpressionsInStatement.push(node);
+        },
+        CallExpression: (node) => {
+            callExpressionsInStatement.push(node);
+        },
         MemberExpression: (node) => {
             if (!isInsideReachableMethod() || isInsideSkippedBlock()) {
                 return;
             }
+
+            memberExpressionsInStatement.push(node);
+
             if (
                 node.object.type === 'ThisExpression' &&
+                globalAccessQualifiers.has(node.parent.type) &&
                 node.property.type === 'Identifier' &&
                 forbiddenPropertyNames.includes(node.property.name)
             ) {
-                reporter(node);
+                // Prevents expressions like:
+                // this.dispatchEvent(new CustomEvent('myevent'));
+                // this.querySelector('button').addEventListener('click', ...);
+                // this.querySelector?.('button').addEventListener('click', ...);
+                // this.querySelector?.('button')?.addEventListener('click', ...);
+                // this.querySelector?.('button').firstElementChild.id;
+                // this.childNodes.item(0).textContent = 'foo';
+
+                // Allows all-optional expressions like:
+                // this.dispatchEvent?.(new CustomEvent('myevent'));
+                // this.querySelector?.('button')?.addEventListener?.('click', ...);
+                // this.querySelector?.('button')?.firstElementChild.id;
+                const allCallExpressionsOptional = callExpressionsInStatement.every(
+                    (expression) => expression.optional,
+                );
+                const allMemberExpressionsOptional = memberExpressionsInStatement.every(
+                    (expression, index) => {
+                        if (expression.parent && expression.parent.type === 'CallExpression') {
+                            // Skip CallExpressions here as they are treated separately
+                            return true;
+                        }
+                        // Return `true` if the MemberExpression is either `optional` or
+                        // the last expression of the chain (which is in revered order).
+                        return expression.optional || index === 0;
+                    },
+                );
+                if (!allCallExpressionsOptional || !allMemberExpressionsOptional) {
+                    reporter(node);
+                }
             }
         },
     };

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

@@ -9,6 +9,9 @@ const { noPropertyAccessDuringSSR } = require('../rule-helpers');
 const { docUrl } = require('../util/doc-url');
 
 const disallowedProperties = [
+    'attachInternals',
+    'children',
+    'childNodes',
     'dispatchEvent',
     'firstChild',
     'firstElementChild',
@@ -20,7 +23,6 @@ const disallowedProperties = [
     'ownerDocument',
     'querySelector',
     'querySelectorAll',
-    'removeEventListener',
 ];
 module.exports = {
     meta: {
@@ -32,7 +34,8 @@ module.exports = {
         },
         schema: [],
         messages: {
-            propertyAccessFound: '`{{ identifier }}` is unsupported in SSR.',
+            propertyAccessFound:
+                '`{{ identifier }}` is unsupported in SSR. Consider guarding access to `{{identifier}}`, e.g. via the `import.meta.env.SSR` flag, or optional chaining (`this.{{identifier}}?.`).',
         },
     },
     create: (context) => {

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

@@ -13,7 +13,7 @@ const rule = require('../../../lib/rules/consistent-component-name');
 
 const ruleTester = new RuleTester(ESLINT_TEST_CONFIG);
 
-ruleTester.run('consistent-class-name', rule, {
+ruleTester.run('consistent-component-name', rule, {
     valid: [
         {
             code: `

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

@@ -192,6 +192,18 @@ tester.run('no-browser-globals-during-ssr', rule, {
               }
             `,
         },
+        {
+            code: `
+              let name;
+              let screen = 'mobile';
+              export default class Foo extends LightningElement {
+                connectedCallback() {
+                  name = "hello";
+                  screen;
+                }
+              }
+            `,
+        },
         {
             code: `
               import { name } from 'some/component';
@@ -639,5 +651,22 @@ tester.run('no-browser-globals-during-ssr', rule, {
                 },
             ],
         },
+        {
+            code: `
+              import { LightningElement } from 'lwc';
+
+              export default class Foo extends LightningElement {
+                connectedCallback() {
+                  name = 'hello';
+                }
+              }
+          `,
+            errors: [
+                {
+                    messageId: 'prohibitedBrowserAPIUsage',
+                    data: { identifier: 'name' },
+                },
+            ],
+        },
     ],
 });