From 95149682b3fca9c1b65968452ebe53f43d3e4f59 Mon Sep 17 00:00:00 2001 From: Sebastian Landwehr Date: Wed, 14 Apr 2021 21:24:37 +0200 Subject: [PATCH 1/4] fix: prefer relative import if a subpath is imported --- lib/helpers/create-fixer.js | 27 ----------- lib/rules/use-alias.js | 73 ++++++++++++++++-------------- tests/helpers/create-fixer.test.js | 11 ----- tests/lib/rules/use-alias.spec.js | 42 +++++++++++------ 4 files changed, 68 insertions(+), 85 deletions(-) delete mode 100644 lib/helpers/create-fixer.js delete mode 100644 tests/helpers/create-fixer.test.js diff --git a/lib/helpers/create-fixer.js b/lib/helpers/create-fixer.js deleted file mode 100644 index e4ddff6..0000000 --- a/lib/helpers/create-fixer.js +++ /dev/null @@ -1,27 +0,0 @@ -const path = require('path') -const entries = Object.entries || ((obj) => Object.keys(obj).map((key) => [key, obj[key]])) - -const createFixer = (options = {}) => { - const { alias, filePath, cwd, node = {} } = options - - let source = node.source // ImportDeclaration or ImportExpression - if (Array.isArray(node.arguments)) { - source = node.arguments[0] // CallExpression - } - - if (!source) return null - - const aliasPaths = entries(alias).map(([key, value]) => [key, path.join(cwd, value)]) - const resolvedPath = path.resolve(filePath, source.value) - const [aliasMatch, aliasPath] = aliasPaths.find(([_, aliasPath]) => resolvedPath.includes(aliasPath)) || [] - - if (!aliasMatch || !aliasPath) return null - - const newPath = resolvedPath.replace(aliasPath, '') - - const replacement = path.join(aliasMatch, newPath) - - return (fixer) => fixer.replaceTextRange([source.range[0] + 1, source.range[1] - 1], replacement) -} - -module.exports = createFixer diff --git a/lib/rules/use-alias.js b/lib/rules/use-alias.js index bd6505a..db7d5cb 100644 --- a/lib/rules/use-alias.js +++ b/lib/rules/use-alias.js @@ -5,9 +5,7 @@ const fs = require('fs') const checkIgnoreDepth = require('../helpers/check-ignore-depth') const findProjectRoot = require('../helpers/find-project-root') const getProperties = require('../helpers/get-properties') -const createFixer = require('../helpers/create-fixer') -const values = Object.values || ((obj) => Object.keys(obj).map((e) => obj[e])) const checkPath = (path, ext) => fs.existsSync(`${path}${ext}`) || fs.existsSync(`${path}/index${ext}`) module.exports = { @@ -72,7 +70,7 @@ module.exports = { return plugin } }) - alias = moduleResolver.options.alias || {} + alias = { ...moduleResolver.options.alias } } catch (error) { const message = 'Unable to find config for babel-plugin-module-resolver' return { @@ -85,11 +83,14 @@ module.exports = { } } - // Build array of alias paths. + // Resolve alias paths. const cwd = projectRootAbsolutePath || process.cwd() - const aliasPaths = values(alias).map((a) => path.join(cwd, a)) + for (const name in alias) { + alias[name] = path.join(cwd, alias[name]) + } - const hasError = (val) => { + const run = ({ node, source }) => { + const val = source.value if (!val) return false // template literals will have undefined val const { ignoreDepth, projectRoot, extensions, allowDepthMoreOrLessThanEquality } = options @@ -99,14 +100,21 @@ module.exports = { // Error if projectRoot option specified but cannot be resolved. if (projectRoot && !projectRootAbsolutePath) { - return { - suggestFix: false, + return context.report({ + node, message: 'Invalid project root specified', - } + loc: source.loc, + }) } - const resolvedPath = path.resolve(filePath, val) + const containedAlias = Object.keys(alias).find((alias) => val.startsWith(alias)) //new Regex(`^${alias}(\\/|$)`).test(val)) + const resolvedPath = containedAlias + ? val.replace(containedAlias, alias[containedAlias]) + : path.resolve(filePath, val) + const [insideAlias] = + Object.entries(alias).find(([aliasName, aliasPath]) => resolvedPath.startsWith(aliasPath)) || [] const resolvedExt = path.extname(val) ? '' : '.js' + const isSubpath = resolvedPath.startsWith(filePath) let pathExists = checkPath(resolvedPath, resolvedExt) @@ -114,43 +122,42 @@ module.exports = { pathExists = extensions.filter((ext) => checkPath(resolvedPath, ext)).length } - const isAliased = aliasPaths.some((aliasPath) => resolvedPath.includes(aliasPath)) - - const error = isAliased && pathExists && val.match(/\.\.\//) // matches, exists, and starts with ../, - return error && { suggestFix: true, message: 'Do not use relative path for aliased modules' } - } - - const reportError = ({ node, source, error }) => { - context.report({ - node, - message: error.message, - loc: source.loc, - fix: error.suggestFix && createFixer({ alias, filePath, cwd, node }), - }) + if (insideAlias && pathExists && val.match(/\.\.\//)) { + // matches, exists, and starts with ../, + const newPath = resolvedPath.replace(alias[insideAlias], '') + const replacement = path.join(insideAlias, newPath).replace(/\\/g, '/') + context.report({ + node, + message: 'Do not use relative path for aliased modules', + loc: source.loc, + fix: (fixer) => fixer.replaceTextRange([source.range[0] + 1, source.range[1] - 1], replacement), + }) + } else if (containedAlias && isSubpath) { + const replacement = `./${path.relative(filePath, resolvedPath)}` + context.report({ + node, + message: 'Do not use aliased path for subpath import', + loc: source.loc, + fix: (fixer) => fixer.replaceTextRange([source.range[0] + 1, source.range[1] - 1], replacement), + }) + } } return { ImportDeclaration(node) { - const error = hasError(node.source.value) - if (error) { - reportError({ node, source: node.source, error }) - } + run({ node, source: node.source }) }, CallExpression(node) { const val = node.callee.name || node.callee.type if (val === 'Import' || val === 'require') { - const error = hasError(node.arguments[0].value) - error && reportError({ node, source: node.arguments[0], error }) + run({ node, source: node.arguments[0] }) } }, ImportExpression(node) { // dynamic import was erroneously using visitorKey for // call expressions https://github.com/babel/babel/pull/10828 // adding ImportExpression for new versions of @babel/eslint-parser - const error = hasError(node.source.value) - if (error) { - reportError({ node, source: node.source, error }) - } + run({ node, source: node.source }) }, } }, diff --git a/tests/helpers/create-fixer.test.js b/tests/helpers/create-fixer.test.js deleted file mode 100644 index d95dea0..0000000 --- a/tests/helpers/create-fixer.test.js +++ /dev/null @@ -1,11 +0,0 @@ -const createFixer = require('../../lib/helpers/create-fixer') - -describe('createFixer', () => { - it('is a function', () => { - expect(createFixer).toBeInstanceOf(Function) - }) - - it('returns a null if no options are passed in', () => { - expect(createFixer()).toBe(null) - }) -}) diff --git a/tests/lib/rules/use-alias.spec.js b/tests/lib/rules/use-alias.spec.js index ba725ec..2cd8e02 100644 --- a/tests/lib/rules/use-alias.spec.js +++ b/tests/lib/rules/use-alias.spec.js @@ -63,18 +63,17 @@ describe('with babel config', () => { ruleTester.run('module-resolver', rule, { valid: [ - "require('actions/api')", - "require('reducers/api')", - "require('ClientMain/api')", - "const { api } = require('actions/api')", - "const { api } = require('reducers/api')", - "import('actions/api')", - "import('reducers/api')", + { code: "require('actions/api')", filename: `${projectRoot}/src/index.js` }, + { code: "require('reducers/api')", filename: `${projectRoot}/src/index.js` }, + { code: "const { api } = require('actions/api')", filename: `${projectRoot}/src/index.js` }, + { code: "const { api } = require('reducers/api')", filename: `${projectRoot}/src/index.js` }, + { code: "import('actions/api')", filename: `${projectRoot}/src/index.js` }, + { code: "import('reducers/api')", filename: `${projectRoot}/src/index.js` }, 'import(`${buildPath}/dist`)', - "import { api } from 'actions/api'", - "import { api } from 'reducers/api'", - "const { api } = dynamic(import('actions/api'))", - "const { api } = dynamic(import('reducers/api'))", + { code: "import { api } from 'actions/api'", filename: `${projectRoot}/src/index.js` }, + { code: "import { api } from 'reducers/api'", filename: `${projectRoot}/src/index.js` }, + { code: "const { api } = dynamic(import('actions/api'))", filename: `${projectRoot}/src/index.js` }, + { code: "const { api } = dynamic(import('reducers/api'))", filename: `${projectRoot}/src/index.js` }, 'const { server } = require(`${buildPath}/dist`)', "const { api } = require('./actions/api')", "const { api } = require('./reducers/api')", @@ -101,12 +100,20 @@ describe('with babel config', () => { ], invalid: [ - createInvalid({ code: "require('../actions/api')", type: 'CallExpression', output: "require('actions/api')" }), - createInvalid({ code: "require('../reducers/api')", type: 'CallExpression', output: "require('reducers/api')" }), + createInvalid({ + code: "require('../actions/api')", + type: 'CallExpression', + output: "require('actions/api')", + }), + createInvalid({ + code: "require('../reducers/api')", + type: 'CallExpression', + output: "require('reducers/api')", + }), createInvalid({ code: "import('../../actions/api')", - type: 'ImportExpression', filename: `${projectRoot}/src/client/index.js`, + type: 'ImportExpression', output: "import('actions/api')", }), createInvalid({ @@ -115,6 +122,13 @@ describe('with babel config', () => { filename: `${projectRoot}/src/client/index.js`, output: "import('reducers/api')", }), + createInvalid({ + code: "import 'actions/api'", + filename: `${projectRoot}/index.js`, + type: 'ImportDeclaration', + output: "import './actions/api'", + errorMessage: 'Do not use aliased path for subpath import', + }), createInvalid({ code: "const { api } = dynamic(import('../actions/api'))", type: 'ImportExpression', From 87f2c73ad5a628520ee2db17d1d7e221db7259e4 Mon Sep 17 00:00:00 2001 From: Sebastian Landwehr Date: Wed, 14 Apr 2021 21:25:54 +0200 Subject: [PATCH 2/4] fix: windows compatibility --- lib/rules/use-alias.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rules/use-alias.js b/lib/rules/use-alias.js index db7d5cb..fdb2d5e 100644 --- a/lib/rules/use-alias.js +++ b/lib/rules/use-alias.js @@ -133,7 +133,7 @@ module.exports = { fix: (fixer) => fixer.replaceTextRange([source.range[0] + 1, source.range[1] - 1], replacement), }) } else if (containedAlias && isSubpath) { - const replacement = `./${path.relative(filePath, resolvedPath)}` + const replacement = `./${path.relative(filePath, resolvedPath).replace(/\\/g, '/')}` context.report({ node, message: 'Do not use aliased path for subpath import', From 02060f49e34093a2d1ff8c87ac71ec40c02bc5e3 Mon Sep 17 00:00:00 2001 From: Sebastian Landwehr Date: Wed, 14 Apr 2021 21:29:09 +0200 Subject: [PATCH 3/4] chore: add test --- tests/lib/rules/use-alias.spec.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/lib/rules/use-alias.spec.js b/tests/lib/rules/use-alias.spec.js index 2cd8e02..9d15651 100644 --- a/tests/lib/rules/use-alias.spec.js +++ b/tests/lib/rules/use-alias.spec.js @@ -122,6 +122,11 @@ describe('with babel config', () => { filename: `${projectRoot}/src/client/index.js`, output: "import('reducers/api')", }), + createInvalid({ + code: "import('./../actions/api')", + type: 'ImportExpression', + output: "import('actions/api')", + }), createInvalid({ code: "import 'actions/api'", filename: `${projectRoot}/index.js`, From 9cb1dbf6791c96fcbb6b97b440ab69cb7a42dfc6 Mon Sep 17 00:00:00 2001 From: Sebastian Landwehr Date: Wed, 14 Apr 2021 21:47:56 +0200 Subject: [PATCH 4/4] fix: use babel-plugin-module-resolver to resolve the aliases --- lib/rules/use-alias.js | 19 ++++++----- package.json | 5 ++- tests/lib/rules/use-alias.spec.js | 7 ++++ yarn.lock | 57 +++++++++++++++++++++++++++++-- 4 files changed, 76 insertions(+), 12 deletions(-) diff --git a/lib/rules/use-alias.js b/lib/rules/use-alias.js index fdb2d5e..17584a7 100644 --- a/lib/rules/use-alias.js +++ b/lib/rules/use-alias.js @@ -1,6 +1,7 @@ const babel = require('@babel/core') const path = require('path') const fs = require('fs') +const { resolvePath } = require('babel-plugin-module-resolver') const checkIgnoreDepth = require('../helpers/check-ignore-depth') const findProjectRoot = require('../helpers/find-project-root') @@ -70,7 +71,7 @@ module.exports = { return plugin } }) - alias = { ...moduleResolver.options.alias } + alias = moduleResolver.options.alias } catch (error) { const message = 'Unable to find config for babel-plugin-module-resolver' return { @@ -85,9 +86,9 @@ module.exports = { // Resolve alias paths. const cwd = projectRootAbsolutePath || process.cwd() - for (const name in alias) { - alias[name] = path.join(cwd, alias[name]) - } + const resolvedAlias = Object.fromEntries( + Object.entries(alias).map(([aliasName, aliasPath]) => [aliasName, path.join(cwd, aliasPath)]), + ) const run = ({ node, source }) => { const val = source.value @@ -107,12 +108,12 @@ module.exports = { }) } - const containedAlias = Object.keys(alias).find((alias) => val.startsWith(alias)) //new Regex(`^${alias}(\\/|$)`).test(val)) + const containedAlias = Object.keys(resolvedAlias).find((name) => val.startsWith(name)) //new Regex(`^${alias}(\\/|$)`).test(val)) const resolvedPath = containedAlias - ? val.replace(containedAlias, alias[containedAlias]) + ? val.replace(containedAlias, resolvedAlias[containedAlias]) : path.resolve(filePath, val) const [insideAlias] = - Object.entries(alias).find(([aliasName, aliasPath]) => resolvedPath.startsWith(aliasPath)) || [] + Object.entries(resolvedAlias).find(([aliasName, aliasPath]) => resolvedPath.startsWith(aliasPath)) || [] const resolvedExt = path.extname(val) ? '' : '.js' const isSubpath = resolvedPath.startsWith(filePath) @@ -124,7 +125,7 @@ module.exports = { if (insideAlias && pathExists && val.match(/\.\.\//)) { // matches, exists, and starts with ../, - const newPath = resolvedPath.replace(alias[insideAlias], '') + const newPath = resolvedPath.replace(resolvedAlias[insideAlias], '') const replacement = path.join(insideAlias, newPath).replace(/\\/g, '/') context.report({ node, @@ -133,7 +134,7 @@ module.exports = { fix: (fixer) => fixer.replaceTextRange([source.range[0] + 1, source.range[1] - 1], replacement), }) } else if (containedAlias && isSubpath) { - const replacement = `./${path.relative(filePath, resolvedPath).replace(/\\/g, '/')}` + const replacement = resolvePath(val, filename, { alias }) context.report({ node, message: 'Do not use aliased path for subpath import', diff --git a/package.json b/package.json index 5146453..4b7285c 100644 --- a/package.json +++ b/package.json @@ -59,5 +59,8 @@ "regenerator-runtime": "^0.13.3", "rimraf": "^3.0.0" }, - "license": "MIT" + "license": "MIT", + "dependencies": { + "babel-plugin-module-resolver": "^4.1.0" + } } diff --git a/tests/lib/rules/use-alias.spec.js b/tests/lib/rules/use-alias.spec.js index 9d15651..be2e999 100644 --- a/tests/lib/rules/use-alias.spec.js +++ b/tests/lib/rules/use-alias.spec.js @@ -134,6 +134,13 @@ describe('with babel config', () => { output: "import './actions/api'", errorMessage: 'Do not use aliased path for subpath import', }), + createInvalid({ + code: "import 'actions/api'", + filename: `${projectRoot}/actions/index.js`, + type: 'ImportDeclaration', + output: "import './api'", + errorMessage: 'Do not use aliased path for subpath import', + }), createInvalid({ code: "const { api } = dynamic(import('../actions/api'))", type: 'ImportExpression', diff --git a/yarn.lock b/yarn.lock index f8b7d47..c43c748 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1533,6 +1533,17 @@ babel-plugin-jest-hoist@^26.6.2: "@types/babel__core" "^7.0.0" "@types/babel__traverse" "^7.0.6" +babel-plugin-module-resolver@^4.1.0: + version "4.1.0" + resolved "https://registry.yarnpkg.com/babel-plugin-module-resolver/-/babel-plugin-module-resolver-4.1.0.tgz#22a4f32f7441727ec1fbf4967b863e1e3e9f33e2" + integrity sha512-MlX10UDheRr3lb3P0WcaIdtCSRlxdQsB1sBqL7W0raF070bGl1HQQq5K3T2vf2XAYie+ww+5AKC/WrkjRO2knA== + dependencies: + find-babel-config "^1.2.0" + glob "^7.1.6" + pkg-up "^3.1.0" + reselect "^4.0.0" + resolve "^1.13.1" + babel-preset-current-node-syntax@^1.0.0: version "1.0.1" resolved "https://registry.yarnpkg.com/babel-preset-current-node-syntax/-/babel-preset-current-node-syntax-1.0.1.tgz#b4399239b89b2a011f9ddbe3e4f401fc40cff73b" @@ -2632,6 +2643,14 @@ fill-range@^7.0.1: dependencies: to-regex-range "^5.0.1" +find-babel-config@^1.2.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/find-babel-config/-/find-babel-config-1.2.0.tgz#a9b7b317eb5b9860cda9d54740a8c8337a2283a2" + integrity sha512-jB2CHJeqy6a820ssiqwrKMeyC6nNdmrcgkKWJWmpoxpE8RKciYJXCcXRq1h2AzCo5I5BJeN2tkGEO3hLTuePRA== + dependencies: + json5 "^0.5.1" + path-exists "^3.0.0" + find-up@^2.0.0, find-up@^2.1.0: version "2.1.0" resolved "https://registry.yarnpkg.com/find-up/-/find-up-2.1.0.tgz#45d1b7e506c717ddd482775a2b77920a3c0c57a7" @@ -2639,6 +2658,13 @@ find-up@^2.0.0, find-up@^2.1.0: dependencies: locate-path "^2.0.0" +find-up@^3.0.0: + version "3.0.0" + resolved "https://registry.yarnpkg.com/find-up/-/find-up-3.0.0.tgz#49169f1d7993430646da61ecc5ae355c21c97b73" + integrity sha512-1yD6RmLI1XBfxugvORwlck6f75tYL+iR0jqwsOrOxMZyGYqUuDhJ0l4AXdO1iX/FTs9cBAMEk1gWSEx1kSbylg== + dependencies: + locate-path "^3.0.0" + find-up@^4.0.0, find-up@^4.1.0: version "4.1.0" resolved "https://registry.yarnpkg.com/find-up/-/find-up-4.1.0.tgz#97afe7d6cdc0bc5928584b7c8d7b16e8a9aa5d19" @@ -2798,7 +2824,7 @@ glob-parent@^5.0.0, glob-parent@~5.1.0: dependencies: is-glob "^4.0.1" -glob@^7.0.0, glob@^7.1.1, glob@^7.1.2, glob@^7.1.3, glob@^7.1.4: +glob@^7.0.0, glob@^7.1.1, glob@^7.1.2, glob@^7.1.3, glob@^7.1.4, glob@^7.1.6: version "7.1.6" resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.6.tgz#141f33b81a7c2492e125594307480c46679278a6" integrity sha512-LwaxwyZ72Lk7vZINtNNrywX0ZuLyStrdDtabefZKAY5ZGJhVtgdznluResxNmPitE0SAO+O26sWTHeKSI2wMBA== @@ -3968,6 +3994,14 @@ locate-path@^2.0.0: p-locate "^2.0.0" path-exists "^3.0.0" +locate-path@^3.0.0: + version "3.0.0" + resolved "https://registry.yarnpkg.com/locate-path/-/locate-path-3.0.0.tgz#dbec3b3ab759758071b58fe59fc41871af21400e" + integrity sha512-7AO748wWnIhNqAuaty2ZWHkQHRSNfPVIsPIfwEOWO22AmaoVrWavlOcMR5nzTLNYvp36X220/maaRsrec1G65A== + dependencies: + p-locate "^3.0.0" + path-exists "^3.0.0" + locate-path@^5.0.0: version "5.0.0" resolved "https://registry.yarnpkg.com/locate-path/-/locate-path-5.0.0.tgz#1afba396afd676a6d42504d0a67a3a7eb9f62aa0" @@ -4364,7 +4398,7 @@ p-limit@^1.1.0: dependencies: p-try "^1.0.0" -p-limit@^2.2.0: +p-limit@^2.0.0, p-limit@^2.2.0: version "2.3.0" resolved "https://registry.yarnpkg.com/p-limit/-/p-limit-2.3.0.tgz#3dd33c647a214fdfffd835933eb086da0dc21db1" integrity sha512-//88mFWSJx8lxCzwdAABTJL2MyWB12+eIY7MDL2SqLmAkeKU9qxRvWuSyTjm3FUmpBEMuFfckAIqEaVGUDxb6w== @@ -4378,6 +4412,13 @@ p-locate@^2.0.0: dependencies: p-limit "^1.1.0" +p-locate@^3.0.0: + version "3.0.0" + resolved "https://registry.yarnpkg.com/p-locate/-/p-locate-3.0.0.tgz#322d69a05c0264b25997d9f40cd8a891ab0064a4" + integrity sha512-x+12w/To+4GFfgJhBEpiDcLozRJGegY+Ei7/z0tSLkMmxGZNybVMSfWj9aJn8Z5Fc7dBUNJOOVgPv2H7IwulSQ== + dependencies: + p-limit "^2.0.0" + p-locate@^4.1.0: version "4.1.0" resolved "https://registry.yarnpkg.com/p-locate/-/p-locate-4.1.0.tgz#a3428bb7088b3a60292f66919278b7c297ad4f07" @@ -4524,6 +4565,13 @@ pkg-dir@^4.2.0: dependencies: find-up "^4.0.0" +pkg-up@^3.1.0: + version "3.1.0" + resolved "https://registry.yarnpkg.com/pkg-up/-/pkg-up-3.1.0.tgz#100ec235cc150e4fd42519412596a28512a0def5" + integrity sha512-nDywThFk1i4BQK4twPQ6TA4RT8bDY96yeuCVBWL3ePARCiEKDRSrNGbFIgUJpLp+XeIR65v8ra7WuJOFUBtkMA== + dependencies: + find-up "^3.0.0" + please-upgrade-node@^3.2.0: version "3.2.0" resolved "https://registry.yarnpkg.com/please-upgrade-node/-/please-upgrade-node-3.2.0.tgz#aeddd3f994c933e4ad98b99d9a556efa0e2fe942" @@ -4817,6 +4865,11 @@ require-main-filename@^2.0.0: resolved "https://registry.yarnpkg.com/require-main-filename/-/require-main-filename-2.0.0.tgz#d0b329ecc7cc0f61649f62215be69af54aa8989b" integrity sha512-NKN5kMDylKuldxYLSUfrbo5Tuzh4hd+2E8NPPX02mZtn1VuREQToYe/ZdlJy+J3uCpfaiGF05e7B8W0iXbQHmg== +reselect@^4.0.0: + version "4.0.0" + resolved "https://registry.yarnpkg.com/reselect/-/reselect-4.0.0.tgz#f2529830e5d3d0e021408b246a206ef4ea4437f7" + integrity sha512-qUgANli03jjAyGlnbYVAV5vvnOmJnODyABz51RdBN7M4WaVu8mecZWgyQNkG8Yqe3KRGRt0l4K4B3XVEULC4CA== + resolve-cwd@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/resolve-cwd/-/resolve-cwd-3.0.0.tgz#0f0075f1bb2544766cf73ba6a6e2adfebcb13f2d"