diff --git a/packages/code-infra/src/eslint/material-ui/index.mjs b/packages/code-infra/src/eslint/material-ui/index.mjs index 5e21a4082..e2d53733b 100644 --- a/packages/code-infra/src/eslint/material-ui/index.mjs +++ b/packages/code-infra/src/eslint/material-ui/index.mjs @@ -1,3 +1,4 @@ +import consistentProductionGuard from './rules/consistent-production-guard.mjs'; import disallowActiveElementAsKeyEventTarget from './rules/disallow-active-element-as-key-event-target.mjs'; import disallowReactApiInServerComponents from './rules/disallow-react-api-in-server-components.mjs'; import docgenIgnoreBeforeComment from './rules/docgen-ignore-before-comment.mjs'; @@ -5,6 +6,7 @@ import muiNameMatchesComponentName from './rules/mui-name-matches-component-name import noEmptyBox from './rules/no-empty-box.mjs'; import noRestrictedResolvedImports from './rules/no-restricted-resolved-imports.mjs'; import noStyledBox from './rules/no-styled-box.mjs'; +import requireDevWrapper from './rules/require-dev-wrapper.mjs'; import rulesOfUseThemeVariants from './rules/rules-of-use-theme-variants.mjs'; import straightQuotes from './rules/straight-quotes.mjs'; @@ -14,6 +16,7 @@ export default /** @type {import('eslint').ESLint.Plugin} */ ({ version: '0.1.0', }, rules: { + 'consistent-production-guard': consistentProductionGuard, 'disallow-active-element-as-key-event-target': disallowActiveElementAsKeyEventTarget, 'docgen-ignore-before-comment': docgenIgnoreBeforeComment, 'mui-name-matches-component-name': muiNameMatchesComponentName, @@ -23,5 +26,6 @@ export default /** @type {import('eslint').ESLint.Plugin} */ ({ 'straight-quotes': straightQuotes, 'disallow-react-api-in-server-components': disallowReactApiInServerComponents, 'no-restricted-resolved-imports': noRestrictedResolvedImports, + 'require-dev-wrapper': requireDevWrapper, }, }); diff --git a/packages/code-infra/src/eslint/material-ui/rules/consistent-production-guard.mjs b/packages/code-infra/src/eslint/material-ui/rules/consistent-production-guard.mjs new file mode 100644 index 000000000..2d6223332 --- /dev/null +++ b/packages/code-infra/src/eslint/material-ui/rules/consistent-production-guard.mjs @@ -0,0 +1,95 @@ +import { isProcessEnvNodeEnv, isLiteralEq } from './nodeEnvUtils.mjs'; + +/** + * ESLint rule that enforces consistent patterns for production guard checks. + * + * @example + * // Valid - comparing with 'production' + * if (process.env.NODE_ENV !== 'production') {} + * + * @example + * // Valid - comparing with 'production' + * if (process.env.NODE_ENV === 'production') {} + * + * @example + * // Invalid - comparing with 'development' + * if (process.env.NODE_ENV === 'development') {} + * + * @example + * // Invalid - comparing with 'test' + * if (process.env.NODE_ENV !== 'test') {} + * + * @example + * // Invalid - non-static construct + * const env = 'production'; + * if (process.env.NODE_ENV !== env) {} + * + * @example + * // Usage in ESLint config + * { + * rules: { + * 'material-ui/consistent-production-guard': 'error' + * } + * } + * + * @type {import('eslint').Rule.RuleModule} + */ +const rule = { + meta: { + type: 'problem', + docs: { + description: + 'Enforce consistent patterns for production guard checks using process.env.NODE_ENV', + }, + messages: { + invalidComparison: + "Only compare process.env.NODE_ENV with 'production'. Use `process.env.NODE_ENV !== 'production'` or `process.env.NODE_ENV === 'production'` instead of comparing with '{{ comparedValue }}'.", + invalidUsage: + "process.env.NODE_ENV must be used in a binary comparison with === or !== and a literal 'production'. Use `process.env.NODE_ENV !== 'production'` or `process.env.NODE_ENV === 'production'`.", + }, + schema: [], + }, + create(context) { + /** + * @param {import("estree").BinaryExpression} binaryNode + * @param {import("estree").Expression | import("estree").PrivateIdentifier} valueNode + */ + function report(binaryNode, valueNode) { + context.report({ + node: binaryNode, + messageId: 'invalidComparison', + data: { + comparedValue: valueNode.type === 'Literal' ? String(valueNode.value) : 'non-literal', + }, + }); + } + + return { + MemberExpression(node) { + if (isProcessEnvNodeEnv(node)) { + // Check if it's part of a valid binary expression + const parent = node.parent; + if ( + parent && + parent.type === 'BinaryExpression' && + (parent.operator === '===' || parent.operator === '!==') + ) { + if (parent.left === node && !isLiteralEq(parent.right, 'production')) { + report(parent, parent.right); + } else if (parent.right === node && !isLiteralEq(parent.left, 'production')) { + report(parent, parent.left); + } + // Valid usage, do nothing + } else { + context.report({ + node, + messageId: 'invalidUsage', + }); + } + } + }, + }; + }, +}; + +export default rule; diff --git a/packages/code-infra/src/eslint/material-ui/rules/consistent-production-guard.test.mjs b/packages/code-infra/src/eslint/material-ui/rules/consistent-production-guard.test.mjs new file mode 100644 index 000000000..a556e1217 --- /dev/null +++ b/packages/code-infra/src/eslint/material-ui/rules/consistent-production-guard.test.mjs @@ -0,0 +1,162 @@ +import eslint from 'eslint'; +import parser from '@typescript-eslint/parser'; +import rule from './consistent-production-guard.mjs'; + +const ruleTester = new eslint.RuleTester({ + languageOptions: { + parser, + }, +}); + +ruleTester.run('consistent-production-guard', rule, { + valid: [ + // Should pass: Valid !== comparison with 'production' + { + code: ` +if (process.env.NODE_ENV !== 'production') { + console.log('dev'); +} + `, + }, + // Should pass: Valid === comparison with 'production' + { + code: ` +if (process.env.NODE_ENV === 'production') { + console.log('prod'); +} + `, + }, + // Should pass: Reversed comparison (literal on left) + { + code: ` +if ('production' !== process.env.NODE_ENV) { + console.log('dev'); +} + `, + }, + // Should pass: Reversed comparison with === + { + code: ` +if ('production' === process.env.NODE_ENV) { + console.log('prod'); +} + `, + }, + // Should pass: Code without process.env.NODE_ENV + { + code: ` +const foo = 'bar'; +if (foo === 'baz') { + console.log('test'); +} + `, + }, + ], + invalid: [ + // Should fail: Comparing with 'development' + { + code: ` +if (process.env.NODE_ENV === 'development') { + console.log('dev'); +} + `, + errors: [ + { + messageId: 'invalidComparison', + data: { comparedValue: 'development' }, + }, + ], + }, + // Should fail: Comparing with 'test' + { + code: ` +if (process.env.NODE_ENV !== 'test') { + console.log('not test'); +} + `, + errors: [ + { + messageId: 'invalidComparison', + data: { comparedValue: 'test' }, + }, + ], + }, + // Should fail: Reversed comparison with 'development' + { + code: ` +if ('development' === process.env.NODE_ENV) { + console.log('dev'); +} + `, + errors: [ + { + messageId: 'invalidComparison', + data: { comparedValue: 'development' }, + }, + ], + }, + // Should fail: Non-static comparison (variable) + { + code: ` +const env = 'production'; +if (process.env.NODE_ENV !== env) { + console.log('check'); +} + `, + errors: [ + { + messageId: 'invalidComparison', + data: { comparedValue: 'non-literal' }, + }, + ], + }, + // Should fail: Non-static comparison (reversed) + { + code: ` +const env = 'production'; +if (env === process.env.NODE_ENV) { + console.log('check'); +} + `, + errors: [ + { + messageId: 'invalidComparison', + data: { comparedValue: 'non-literal' }, + }, + ], + }, + // Should fail: Invalid usage (function call) + { + code: ` +foo(process.env.NODE_ENV); + `, + errors: [ + { + messageId: 'invalidUsage', + }, + ], + }, + // Should fail: Invalid usage (assignment) + { + code: ` +const env = process.env.NODE_ENV; + `, + errors: [ + { + messageId: 'invalidUsage', + }, + ], + }, + // Should fail: Invalid usage (template literal) + { + code: ` +const message = \`Environment: \${process.env.NODE_ENV}\`; + `, + errors: [ + { + messageId: 'invalidUsage', + }, + ], + }, + ], +}); diff --git a/packages/code-infra/src/eslint/material-ui/rules/nodeEnvUtils.mjs b/packages/code-infra/src/eslint/material-ui/rules/nodeEnvUtils.mjs new file mode 100644 index 000000000..ec972f1bc --- /dev/null +++ b/packages/code-infra/src/eslint/material-ui/rules/nodeEnvUtils.mjs @@ -0,0 +1,41 @@ +/** + * Shared utilities for ESLint rules dealing with process.env.NODE_ENV + */ + +/** + * Checks if a node is process.env.NODE_ENV + * @param {import('estree').Node} node + * @returns {boolean} + */ +export function isProcessEnvNodeEnv(node) { + return ( + node.type === 'MemberExpression' && + node.object.type === 'MemberExpression' && + node.object.object.type === 'Identifier' && + node.object.object.name === 'process' && + node.object.property.type === 'Identifier' && + node.object.property.name === 'env' && + node.property.type === 'Identifier' && + node.property.name === 'NODE_ENV' + ); +} + +/** + * Checks if a node is a Literal with a specific value + * @param {import('estree').Node} node + * @param {string} value + * @returns {boolean} + */ +export function isLiteralEq(node, value) { + return node.type === 'Literal' && node.value === value; +} + +/** + * Checks if a node is a Literal with a value not equal to the specified one + * @param {import('estree').Node} node + * @param {string} value + * @returns {boolean} + */ +export function isLiteralNeq(node, value) { + return node.type === 'Literal' && node.value !== value; +} diff --git a/packages/code-infra/src/eslint/material-ui/rules/require-dev-wrapper.mjs b/packages/code-infra/src/eslint/material-ui/rules/require-dev-wrapper.mjs new file mode 100644 index 000000000..70e892b5c --- /dev/null +++ b/packages/code-infra/src/eslint/material-ui/rules/require-dev-wrapper.mjs @@ -0,0 +1,152 @@ +import { isProcessEnvNodeEnv, isLiteralEq, isLiteralNeq } from './nodeEnvUtils.mjs'; + +/** + * ESLint rule that enforces certain function calls to be wrapped with + * a production check to prevent them from ending up in production bundles. + * + * @example + * // Valid - function wrapped with production check + * if (process.env.NODE_ENV !== 'production') { + * checkSlot(key, overrides[k]); + * } + * + * @example + * // Invalid - function not wrapped + * checkSlot(key, overrides[k]); // Will trigger error + * + * @example + * // Usage in ESLint config + * { + * rules: { + * 'material-ui/require-dev-wrapper': ['error', { + * functionNames: ['warnOnce', 'warn', 'checkSlot'] + * }] + * } + * } + * + * @type {import('eslint').Rule.RuleModule} + */ +const rule = { + meta: { + type: 'problem', + docs: { + description: + 'Enforce that certain function calls are wrapped with a production check to prevent them from ending up in production bundles', + }, + messages: { + missingDevWrapper: + "Function `{{ functionName }}` must be wrapped with a production check (e.g., `if (process.env.NODE_ENV !== 'production')`) to prevent it from ending up in production bundles.", + }, + schema: [ + { + type: 'object', + properties: { + functionNames: { + type: 'array', + items: { + type: 'string', + }, + default: ['warnOnce', 'warn', 'checkSlot'], + }, + }, + additionalProperties: false, + }, + ], + }, + create(context) { + const options = context.options[0] || {}; + const functionNames = options.functionNames || ['warnOnce', 'warn', 'checkSlot']; + + /** + * Checks if a binary expression is comparing process.env.NODE_ENV appropriately + * @param {import('estree').BinaryExpression} binaryExpression - The binary expression to check + * @param {string} operator - The expected comparison operator (===, !==, etc.) + * @param {string} value - The value to compare with + * @returns {boolean} + */ + function isNodeEnvComparison(binaryExpression, operator, value) { + const { left, right } = binaryExpression; + + // Check for exact match with the specified value + if ( + binaryExpression.operator === operator && + ((isProcessEnvNodeEnv(left) && isLiteralEq(right, value)) || + (isProcessEnvNodeEnv(right) && isLiteralEq(left, value))) + ) { + return true; + } + + // For !== operator also allow === with any literal value that's NOT 'production' + if ( + operator === '!==' && + binaryExpression.operator === '===' && + ((isProcessEnvNodeEnv(left) && isLiteralNeq(right, value)) || + (isProcessEnvNodeEnv(right) && isLiteralNeq(left, value))) + ) { + return true; + } + + return false; + } + + /** + * Checks if a node is wrapped in any production check conditional + * @param {import('estree').Node & import('eslint').Rule.NodeParentExtension} node + * @returns {boolean} + */ + function isWrappedInProductionCheck(node) { + let current = node.parent; + let currentChild = node; + + while (current) { + // Check if we're inside an if statement + if (current.type === 'IfStatement') { + // Determine which branch we're in + const isInConsequent = current.consequent === currentChild; + const isInAlternate = current.alternate === currentChild; + + // Skip if not in a branch + if (isInConsequent || isInAlternate) { + const test = current.test; + + // If we're in the consequent, we need !== + // If we're in the alternate (else), we need === + const operator = isInConsequent ? '!==' : '==='; + + // Check for the specific pattern with the right operator + if ( + test.type === 'BinaryExpression' && + isNodeEnvComparison(test, operator, 'production') + ) { + return true; + } + } + } + + currentChild = current; + current = current.parent; + } + + return false; + } + + return { + CallExpression(node) { + // Check if the callee is one of the restricted function names + if (node.callee.type === 'Identifier' && functionNames.includes(node.callee.name)) { + if (!isWrappedInProductionCheck(node)) { + context.report({ + node, + messageId: 'missingDevWrapper', + data: { + functionName: node.callee.name, + }, + }); + } + } + }, + }; + }, +}; + +export default rule; diff --git a/packages/code-infra/src/eslint/material-ui/rules/require-dev-wrapper.test.mjs b/packages/code-infra/src/eslint/material-ui/rules/require-dev-wrapper.test.mjs new file mode 100644 index 000000000..f10f1ecb0 --- /dev/null +++ b/packages/code-infra/src/eslint/material-ui/rules/require-dev-wrapper.test.mjs @@ -0,0 +1,232 @@ +import eslint from 'eslint'; +import parser from '@typescript-eslint/parser'; +import rule from './require-dev-wrapper.mjs'; + +const ruleTester = new eslint.RuleTester({ + languageOptions: { + parser, + }, +}); + +ruleTester.run('require-dev-wrapper', rule, { + valid: [ + // Should pass: Function wrapped with !== production check + { + code: ` +if (process.env.NODE_ENV !== 'production') { + checkSlot(key, overrides[k]); +} + `, + }, + // Should pass: Function wrapped with === 'development' check (tree-shakes in production) + { + code: ` +if (process.env.NODE_ENV === 'development') { + checkSlot(key, overrides[k]); +} + `, + }, + // Should pass: Function wrapped in a for loop inside production check + { + code: ` +if (process.env.NODE_ENV !== 'production') { + for (key in slots) { + checkSlot(key, overrides[k]); + } +} + `, + }, + // Should pass: Other functions not in the list + { + code: ` +otherFunction('hello'); + `, + }, + // Should pass: warnOnce wrapped correctly + { + code: ` +if (process.env.NODE_ENV !== 'production') { + warnOnce('Some warning message'); +} + `, + }, + // Should pass: Multiple statements in production check + { + code: ` +if (process.env.NODE_ENV !== 'production') { + const message = 'Warning'; + warn(message); + checkSlot(key, value); +} + `, + }, + // Should pass: Nested blocks + { + code: ` +if (process.env.NODE_ENV !== 'production') { + if (someCondition) { + warnOnce('nested warning'); + } +} + `, + }, + // Should pass: Reversed comparison (literal on left) + { + code: ` +if ('production' !== process.env.NODE_ENV) { + checkSlot(key, value); +} + `, + }, + // Should pass: Function in else block when if checks for production + { + code: ` +if (process.env.NODE_ENV === 'production') { + // production code +} else { + checkSlot(key, value); +} + `, + }, + // Should pass: Nested if statement in else block + { + code: ` +if (process.env.NODE_ENV === 'production') { + // production code +} else { + if (someCondition) { + warnOnce('nested warning in else'); + } +} + `, + }, + ], + invalid: [ + // Should fail: checkSlot without production check + { + code: ` +checkSlot(key, overrides[k]); + `, + errors: [ + { + messageId: 'missingDevWrapper', + data: { functionName: 'checkSlot' }, + }, + ], + }, + // Should fail: warnOnce without production check + { + code: ` +warnOnce('Some warning'); + `, + errors: [ + { + messageId: 'missingDevWrapper', + data: { functionName: 'warnOnce' }, + }, + ], + }, + // Should fail: warn without production check + { + code: ` +warn('Some warning'); + `, + errors: [ + { + messageId: 'missingDevWrapper', + data: { functionName: 'warn' }, + }, + ], + }, + // Should fail: Multiple unwrapped calls + { + code: ` +checkSlot(key, value); +warn('Warning message'); + `, + errors: [ + { + messageId: 'missingDevWrapper', + data: { functionName: 'checkSlot' }, + }, + { + messageId: 'missingDevWrapper', + data: { functionName: 'warn' }, + }, + ], + }, + // Should fail: Inside wrong conditional (no process.env.NODE_ENV) + { + code: ` +if (someOtherCondition) { + checkSlot(key, value); +} + `, + errors: [ + { + messageId: 'missingDevWrapper', + data: { functionName: 'checkSlot' }, + }, + ], + }, + // Should fail: Inside else block of production check (!== 'production') + { + code: ` +if (process.env.NODE_ENV !== 'production') { + // ok +} else { + checkSlot(key, value); +} + `, + errors: [ + { + messageId: 'missingDevWrapper', + data: { functionName: 'checkSlot' }, + }, + ], + }, + // Should fail: In then block of === 'production' (would run in production!) + { + code: ` +if (process.env.NODE_ENV === 'production') { + checkSlot(key, value); +} + `, + errors: [ + { + messageId: 'missingDevWrapper', + data: { functionName: 'checkSlot' }, + }, + ], + }, + // Should fail: !== 'test' doesn't reliably tree-shake + { + code: ` +if (process.env.NODE_ENV !== 'test') { + checkSlot(key, value); +} + `, + errors: [ + { + messageId: 'missingDevWrapper', + data: { functionName: 'checkSlot' }, + }, + ], + }, + // Should fail: Non-static check doesn't tree-shake + { + code: ` +const env = 'production'; +if (process.env.NODE_ENV !== env) { + checkSlot(key, value); +} + `, + errors: [ + { + messageId: 'missingDevWrapper', + data: { functionName: 'checkSlot' }, + }, + ], + }, + ], +});