diff --git a/docs/rules/no-multi-comp.md b/docs/rules/no-multi-comp.md index 00067c89d6..1e38c10a10 100644 --- a/docs/rules/no-multi-comp.md +++ b/docs/rules/no-multi-comp.md @@ -69,6 +69,55 @@ class HelloJohn extends React.Component { module.exports = HelloJohn; ``` +### `ignoreInternal` + +When `true` the rule will ignore components which are not exported, which allows you to define components that are consumed only within the same file. +This ensures there is only one entry point for a React component without limiting the structural content of the file. + +Examples of **correct** code for this rule: + +```jsx +export function Hello(props) { + return
Hello {props.name}
; +} +function HelloAgain(props) { + return
Hello again {props.name}
; +} +``` + +```jsx +function Hello(props) { + return
Hello {props.name}
; +} +class HelloJohn extends React.Component { + render() { + return ; + } +} +module.exports = HelloJohn; +``` + +Examples of **incorrect** code for this rule: + +```jsx +export function Hello(props) { + return
Hello {props.name}
; +} +export function HelloAgain(props) { + return
Hello again {props.name}
; +} +``` + +```jsx +function Hello(props) { + return
Hello {props.name}
; +} +function HelloAgain(props) { + return
Hello again {props.name}
; +} +module.exports = { Hello, HelloAgain }; +``` + ## When Not To Use It If you prefer to declare multiple components per file you can disable this rule. diff --git a/lib/rules/no-multi-comp.js b/lib/rules/no-multi-comp.js index 8cf73c90bc..2124128875 100644 --- a/lib/rules/no-multi-comp.js +++ b/lib/rules/no-multi-comp.js @@ -17,6 +17,7 @@ const report = require('../util/report'); const messages = { onlyOneComponent: 'Declare only one React component per file', + onlyOneExportedComponent: 'Declare only one exported React component per file', }; /** @type {import('eslint').Rule.RuleModule} */ @@ -38,6 +39,10 @@ module.exports = { default: false, type: 'boolean', }, + ignoreInternal: { + default: false, + type: 'boolean', + }, }, additionalProperties: false, }], @@ -46,6 +51,46 @@ module.exports = { create: Components.detect((context, components, utils) => { const configuration = context.options[0] || {}; const ignoreStateless = configuration.ignoreStateless || false; + const ignoreInternal = configuration.ignoreInternal || false; + + const exportedComponents = new Set(); // Track exported components + const validIdentifiers = new Set(['ArrowFunctionExpression', 'Identifier', 'FunctionExpression']); + + /** + * Given an export declaration, find the export name. + * @param {Object} node + * @returns {string} + */ + function getExportedComponentName(node) { + if (node.declaration.type === 'ClassDeclaration') { + return node.declaration.id.name; + } + if (node.declaration.type === 'Identifier') { + return node.declaration.name; + } + if (node.declaration.declarations) { + const declarator = node.declaration.declarations.find((declaration) => validIdentifiers.has(declaration.init.type)); + if (declarator) { + return declarator.id.name; + } + } + } + + /** + * Given a React component, find the exported name. + * @param {Object} component + * @returns {string} + */ + function findComponentIdentifierFromComponent(component) { + let name; + if (component.node.parent.id) { + name = component.node.parent.id.name; + } + if (!name) { + name = component.node.id.name; + } + return name; + } /** * Checks if the component is ignored @@ -61,21 +106,46 @@ module.exports = { ); } - return { + /** + * Checks if the component is exported, if exportOnly is set + * @param {Object} component The component being checked. + * @returns {boolean} True if the component is exported or exportOnly is false + */ + function isPrivate(component) { + return ignoreInternal && !exportedComponents.has(findComponentIdentifierFromComponent(component)); + } + + const rule = { 'Program:exit'() { if (components.length() <= 1) { return; } values(components.list()) - .filter((component) => !isIgnored(component)) + .filter((component) => !isIgnored(component) && !isPrivate(component)) .slice(1) .forEach((component) => { - report(context, messages.onlyOneComponent, 'onlyOneComponent', { - node: component.node, - }); + report(context, + ignoreInternal ? messages.onlyOneExportedComponent : messages.onlyOneComponent, + ignoreInternal ? 'onlyOneExportedComponent' : 'onlyOneComponent', + { + node: component.node, + }); }); }, }; + + if (ignoreInternal) { + Object.assign(rule, { + ExportNamedDeclaration(node) { + exportedComponents.add(getExportedComponentName(node)); + }, + ExportDefaultDeclaration(node) { + exportedComponents.add(getExportedComponentName(node)); + }, + }); + } + + return rule; }), }; diff --git a/tests/lib/rules/no-multi-comp.js b/tests/lib/rules/no-multi-comp.js index db6f09db2c..bdc5512746 100644 --- a/tests/lib/rules/no-multi-comp.js +++ b/tests/lib/rules/no-multi-comp.js @@ -22,6 +22,178 @@ const parserOptions = { }, }; +// ------------------------------------------------------------------------------ +// Combinatorial test generation for ignoreInvalid +// ------------------------------------------------------------------------------ +// eslint-disable-next-line valid-jsdoc +/** + * @typedef {Function} ComponentGenerator + * @param {string} name - The name of the component to be generated. + * @returns {string} - The component declaration. + */ + +// eslint-disable-next-line valid-jsdoc +/** + * @type {ComponentGenerator[]} Array of different ways to output valid code for instantiating a React component with the given name. + */ +const SIMPLE_COMPONENT_TYPES = [ + (name) => `const ${name} = () => <>;`, // arrow function component + (name) => `let ${name} = () => <>;`, // arrow function component (with let) + (name) => `var ${name} = () => <>;`, // arrow function component (with var) + (name) => `const ${name} = memo(() => <>);`, // memoized anonymous component + (name) => `const ${name} = async () => <>;`, // async component (react server components) +]; + +// eslint-disable-next-line valid-jsdoc +/** + * @type {ComponentGenerator[]} Array of different ways to output valid code for instantiating a React component with the given name. + */ +const COMPLEX_COMPONENT_TYPES = [ + (name) => `function ${name}() { return <>; }`, // standard function component + (name) => `class ${name} extends React.Component { + render() { return <>; } + }`, // class component +]; + +/** + * Helper function for combinatorial testing of no-multi-comp ignoreInternal rule. + * + * @typedef {Function} ComponentExportGenerator + * @param {ComponentGenerator} compOne - Generator function for the first component to export. + * @param {string} compOneName - The name of the first component, which will typically be exported. + * @param {ComponentGenerator} compTwo - Generator function for the second component to export. + * @param {string} compTwoName - The name of the second component. This will be exported in invalid scenarios. + * @param {string} exportRename - A potential rename of the export for the first component, used by some scenarios. + * @returns {string} - A (nearly) complete test case - although we also prepend a generic import string. + */ + +// eslint-disable-next-line valid-jsdoc +/** + * @type {ComponentExportGenerator[]} + */ +const EXPORT_TYPES_VALID = [ + (compOne, compOneName, compTwo, compTwoName) => ` + ${compOne(compOneName)} + ${compTwo(compTwoName)}`, // no export at all + // DECLARATION TIME EXPORTS + (compOne, compOneName, compTwo, compTwoName) => ` + export ${compOne(compOneName)} + ${compTwo(compTwoName)}`, // standard export + (compOne, compOneName, compTwo, compTwoName, exportRename) => ` + ${compOne(compOneName)} + ${compTwo(compTwoName)} + module.exports = { ${exportRename} : ${compOneName} }`, // module export with rename, post declaration + // nb: module export at declaration time will be handled separately + // POST DECLARATION EXPORTS + (compOne, compOneName, compTwo, compTwoName) => ` + ${compOne(compOneName)} + ${compTwo(compTwoName)} + export default ${compOneName}`, // default export, post declaration + (compOne, compOneName, compTwo, compTwoName) => ` + ${compOne(compOneName)} + ${compTwo(compTwoName)} + export { ${compOneName} }`, // export, post declaration + (compOne, compOneName, compTwo, compTwoName) => ` + ${compOne(compOneName)} + ${compTwo(compTwoName)} + module.exports = { ${compOneName} }`, // module export, post declaration + (compOne, compOneName, compTwo, compTwoName, exportRename) => ` + ${compOne(compOneName)} + ${compTwo(compTwoName)} + module.exports = { ${exportRename} : ${compOneName} }`, // module export with rename, post declaration + (compOne, compOneName, compTwo, compTwoName) => ` + ${compOne(compOneName)} + ${compTwo(compTwoName)} + export default function() { return <${compOneName} />; }`, // exporting component with indirection +]; + +// eslint-disable-next-line valid-jsdoc +/** + * Special case: inline `export default` syntax cannot be followed by `const, let, var` + * + * @type {ComponentExportGenerator[]} + */ +const EXPORT_TYPES_VALID_COMPLEX = [ + (compOne, compOneName, compTwo, compTwoName) => ` + export default ${compOne(compOneName)} + ${compTwo(compTwoName)}`, // default export +]; + +// eslint-disable-next-line valid-jsdoc +/** + * @type {ComponentExportGenerator[]} + */ +const EXPORT_TYPES_INVALID = [ + // DECLARATION TIME EXPORTS + (compOne, compOneName, compTwo, compTwoName) => ` + export ${compOne(compOneName)} + export ${compTwo(compTwoName)}`, // standard export + // nb: module export at declaration time will be handled separately + // POST DECLARATION EXPORTS + (compOne, compOneName, compTwo, compTwoName) => ` + ${compOne(compOneName)} + ${compTwo(compTwoName)} + export default ${compOneName} + export { ${compTwoName} }`, // default export, post declaration + (compOne, compOneName, compTwo, compTwoName) => ` + ${compOne(compOneName)} + ${compTwo(compTwoName)} + export { ${compOneName} } + export { ${compTwoName} }`, // export, post declaration + (compOne, compOneName, compTwo, compTwoName) => ` + ${compOne(compOneName)} + ${compTwo(compTwoName)} + export { ${compOneName}, ${compTwoName} }`, // export, post declaration + (compOne, compOneName, compTwo, compTwoName) => ` + ${compOne(compOneName)} + ${compTwo(compTwoName)} + module.exports = { ${compOneName} ${compTwoName} }`, // module export, post declaration +]; + +// eslint-disable-next-line valid-jsdoc +/** + * @type {ComponentExportGenerator[]} + */ +const EXPORT_TYPES_INVALID_COMPLEX = [ + // DECLARATION TIME EXPORTS + (compOne, compOneName, compTwo, compTwoName) => ` + export default ${compOne(compOneName)} + export ${compTwo(compTwoName)}`, // default export +]; + +/** + * @param {ComponentExportGenerator[]} scenarioArray array of scenario generator functions which we will now convert into strings + * @param {ComponentGenerator[]} componentTypes array of components to generate on + * @param {boolean} [invalid] whether generated scenarios should expect to fail + * @returns {{code: string, options: object[], errors: object[]}[]} + */ +const generateScenarios = (scenarioArray, componentTypes, invalid = false) => { + const result = []; + for (const scenario of scenarioArray) { + for (const first of componentTypes) { + for (const second of SIMPLE_COMPONENT_TYPES) { + result.push({ + code: ` + import React, { memo, Component } from 'react'; + ${scenario(first, 'ComponentOne', second, 'ComponentTwo', 'RenamedComponent')}`, + options: [{ ignoreInternal: true }], + errors: invalid ? [{ messageId: 'onlyOneExportedComponent' }] : undefined, + }); + } + } + } + return result; +}; + +const ignoreInternalValidScenarios = [ + ...generateScenarios(EXPORT_TYPES_VALID, [...SIMPLE_COMPONENT_TYPES, ...COMPLEX_COMPONENT_TYPES]), + ...generateScenarios(EXPORT_TYPES_VALID_COMPLEX, COMPLEX_COMPONENT_TYPES), +]; +const ignoreInternalInvalidScenarios = [ + ...generateScenarios(EXPORT_TYPES_INVALID, [...SIMPLE_COMPONENT_TYPES, ...COMPLEX_COMPONENT_TYPES]), + ...generateScenarios(EXPORT_TYPES_INVALID_COMPLEX, COMPLEX_COMPONENT_TYPES), +]; + // ------------------------------------------------------------------------------ // Tests // ------------------------------------------------------------------------------ @@ -265,6 +437,36 @@ ruleTester.run('no-multi-comp', rule, { export default MenuList `, }, + ...ignoreInternalValidScenarios, + { // special case: components declared inside of module export block + code: ` + const ComponentOne = () => <>; + module.exports = { + ComponentTwo() { return <>; } + }; + `, + options: [{ ignoreInternal: true }], + }, + { // basic testing for intersection of ignoreStateless and ignoreInternal + code: ` + export function Hello(props) { + return
Hello {props.name}
; + } + export function HelloAgain(props) { + return
Hello again {props.name}
; + } + `, + options: [{ ignoreStateless: true, ignoreInternal: true }], + }, + { + code: ` + export const HelloComponent = (props) => { + return
; + } + export default React.memo((props, ref) => ); + `, + options: [{ ignoreStateless: true, ignoreInternal: true }], + }, ]), invalid: parsers.all([ @@ -612,5 +814,16 @@ ruleTester.run('no-multi-comp', rule, { }, errors: [{ messageId: 'onlyOneComponent' }], }, + ...ignoreInternalInvalidScenarios, + { // special case: components declared inside of module export block + code: ` + module.exports = { + ComponentOne() { return <>; } + ComponentTwo() { return <>; } + }; + `, + options: [{ ignoreInternal: true }], + errors: [{ messageId: 'onlyOneExportedComponent' }], + }, ]), });