From 74efbf75702cd3714a77074d82f8542bc7ce98c9 Mon Sep 17 00:00:00 2001 From: Nicolaos Moscholios Date: Mon, 2 Sep 2024 13:24:22 +0200 Subject: [PATCH 1/2] `jsx-fragments`: allow `Fragment` syntax option Right now one can specify either `syntax` or `element`, however `element` automatically falls back to using `React.Fragment` which is not supported in the case the variable is not exposed as an UMD global - the case when using es modules. This option allows to use modules instead and import the `Fragment` component from the react library --- lib/rules/jsx-fragments.js | 65 ++++++++++++++++++++++++++++++++++---- 1 file changed, 58 insertions(+), 7 deletions(-) diff --git a/lib/rules/jsx-fragments.js b/lib/rules/jsx-fragments.js index 4dadb076d7..8900768236 100644 --- a/lib/rules/jsx-fragments.js +++ b/lib/rules/jsx-fragments.js @@ -25,6 +25,7 @@ const messages = { fragmentsNotSupported: 'Fragments are only supported starting from React v16.2. ' + 'Please disable the `react/jsx-fragments` rule in `eslint` settings or upgrade your version of React.', preferPragma: 'Prefer {{react}}.{{fragment}} over fragment shorthand', + preferPragmaShort: 'Prefer {{fragment}} over fragment shorthand', preferFragment: 'Prefer fragment shorthand over {{react}}.{{fragment}}', }; @@ -41,7 +42,7 @@ module.exports = { messages, schema: [{ - enum: ['syntax', 'element'], + enum: ['syntax', 'element', 'elementShort'], }], }, @@ -53,6 +54,11 @@ module.exports = { const closeFragShort = ''; const openFragLong = `<${reactPragma}.${fragmentPragma}>`; const closeFragLong = ``; + const openFragMedium = `<${fragmentPragma}>`; + const closeFragMedium = ``; + + let reactImportFound = false; + const reactImports = []; function reportOnReactVersion(node) { if (!testReactVersion(context, '>= 16.2.0')) { @@ -65,20 +71,50 @@ module.exports = { return false; } - function getFixerToLong(jsxFragment) { + function getFixerToLong(jsxFragment, withoutReactPragma) { if (!jsxFragment.closingFragment || !jsxFragment.openingFragment) { // the old TS parser crashes here // TODO: FIXME: can we fake these two descriptors? return null; } return function fix(fixer) { + const closeFrag = withoutReactPragma ? closeFragMedium : closeFragLong; + const openFrag = withoutReactPragma ? openFragMedium : openFragLong; let source = getText(context); - source = replaceNode(source, jsxFragment.closingFragment, closeFragLong); - source = replaceNode(source, jsxFragment.openingFragment, openFragLong); - const lengthDiff = openFragLong.length - getText(context, jsxFragment.openingFragment).length - + closeFragLong.length - getText(context, jsxFragment.closingFragment).length; + source = replaceNode(source, jsxFragment.closingFragment, closeFrag); + source = replaceNode(source, jsxFragment.openingFragment, openFrag); + const lengthDiff = openFrag.length - getText(context, jsxFragment.openingFragment).length + + closeFrag.length - getText(context, jsxFragment.closingFragment).length; const range = jsxFragment.range; - return fixer.replaceTextRange(range, source.slice(range[0], range[1] + lengthDiff)); + + const fixes = []; + + // Insert the import statement at the top of the file if `withoutReactPragma` is true + if (withoutReactPragma) { + const ancestors = context.getAncestors(); + const rootNode = ancestors.length > 0 ? ancestors[0] : jsxFragment; + const reactImport = reactImports.find(importNode => + importNode.specifiers.some(spec => spec.imported && spec.imported.name === fragmentPragma) + ); + + if (!reactImport) { + // No `Fragment` import found, so add it + const existingReactImport = reactImports.find(importNode => importNode.source.value === 'react'); + + if (existingReactImport) { + // If there's already an import from 'react', add `Fragment` to the existing specifiers + const lastSpecifier = existingReactImport.specifiers[existingReactImport.specifiers.length - 1]; + fixes.push(fixer.insertTextAfter(lastSpecifier, `, ${fragmentPragma}`)); + } else { + // Otherwise, add a new import statement at the top + fixes.push(fixer.insertTextBefore(rootNode.body[0], `import { Fragment } from 'react';\n\n`)); + } + } + } + + fixes.push(fixer.replaceTextRange(range, source.slice(range[0], range[1] + lengthDiff))) + + return fixes; }; } @@ -165,12 +201,27 @@ module.exports = { fix: getFixerToLong(node), }); } + + if (configuration === 'elementShort') { + report(context, messages.preferPragmaShort, 'preferPragmaShort', { + node, + data: { + react: reactPragma, + fragment: fragmentPragma, + }, + fix: getFixerToLong(node, true), + }); + } }, ImportDeclaration(node) { if (node.source && node.source.value === 'react') { + reactImports.push(node); + let hasFragment = false; + node.specifiers.forEach((spec) => { if (spec.imported && spec.imported.name === fragmentPragma) { + hasFragment = true; if (spec.local) { fragmentNames.add(spec.local.name); } From d1970123554a955809f98e691d173a1d2e6d96cd Mon Sep 17 00:00:00 2001 From: Nicolaos Moscholios Date: Mon, 2 Sep 2024 11:33:13 +0000 Subject: [PATCH 2/2] fix linter --- lib/rules/jsx-fragments.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/rules/jsx-fragments.js b/lib/rules/jsx-fragments.js index 8900768236..b9cd4962b6 100644 --- a/lib/rules/jsx-fragments.js +++ b/lib/rules/jsx-fragments.js @@ -57,7 +57,6 @@ module.exports = { const openFragMedium = `<${fragmentPragma}>`; const closeFragMedium = ``; - let reactImportFound = false; const reactImports = []; function reportOnReactVersion(node) { @@ -89,13 +88,11 @@ module.exports = { const fixes = []; - // Insert the import statement at the top of the file if `withoutReactPragma` is true + // Insert the import statement at the top of the file if `withoutReactPragma` is true if (withoutReactPragma) { const ancestors = context.getAncestors(); const rootNode = ancestors.length > 0 ? ancestors[0] : jsxFragment; - const reactImport = reactImports.find(importNode => - importNode.specifiers.some(spec => spec.imported && spec.imported.name === fragmentPragma) - ); + const reactImport = reactImports.find(importNode => importNode.specifiers.some((spec) => spec.imported && spec.imported.name === fragmentPragma)); if (!reactImport) { // No `Fragment` import found, so add it @@ -107,12 +104,13 @@ module.exports = { fixes.push(fixer.insertTextAfter(lastSpecifier, `, ${fragmentPragma}`)); } else { // Otherwise, add a new import statement at the top + // eslint-disable-next-line semi fixes.push(fixer.insertTextBefore(rootNode.body[0], `import { Fragment } from 'react';\n\n`)); } } } - fixes.push(fixer.replaceTextRange(range, source.slice(range[0], range[1] + lengthDiff))) + fixes.push(fixer.replaceTextRange(range, source.slice(range[0], range[1] + lengthDiff))); return fixes; }; @@ -217,11 +215,9 @@ module.exports = { ImportDeclaration(node) { if (node.source && node.source.value === 'react') { reactImports.push(node); - let hasFragment = false; node.specifiers.forEach((spec) => { if (spec.imported && spec.imported.name === fragmentPragma) { - hasFragment = true; if (spec.local) { fragmentNames.add(spec.local.name); }