From 8adfe535b4eb3750ee0a3e8faeda9c5ee77c89ce Mon Sep 17 00:00:00 2001 From: Ivan Shmidt Date: Tue, 27 Oct 2020 21:13:23 +0100 Subject: [PATCH] feat: consider files field from package.json (#12) Co-authored-by: blam --- src/rules/no-internal-import.js | 61 +++++++++++++++++-- src/rules/no-relative-import.js | 8 +-- test/fixture/yarn/packages/bar/allowed.js | 0 .../fixture/yarn/packages/bar/nested/.gitkeep | 0 test/fixture/yarn/packages/bar/package.json | 8 ++- test/rules/no-internal-import.test.js | 33 +++++++++- yarn.lock | 12 ++-- 7 files changed, 105 insertions(+), 17 deletions(-) create mode 100644 test/fixture/yarn/packages/bar/allowed.js create mode 100644 test/fixture/yarn/packages/bar/nested/.gitkeep diff --git a/src/rules/no-internal-import.js b/src/rules/no-internal-import.js index d733a75..806cd78 100644 --- a/src/rules/no-internal-import.js +++ b/src/rules/no-internal-import.js @@ -3,23 +3,74 @@ import moduleVisitor, { } from 'eslint-module-utils/moduleVisitor'; import parse from 'parse-package-name'; import getPackages from 'get-monorepo-packages'; +import path from 'path'; +import minimatch from 'minimatch'; +import fs from 'fs'; export const meta = { schema: [makeOptionsSchema({})], }; +const withoutExtension = (importFile, fileEntry) => { + const importExt = path.extname(importFile); + if (importExt !== '') return [importFile, fileEntry]; + + const fileEntryExt = path.extname(fileEntry); + const newFileEntry = + fileEntryExt !== '' + ? fileEntry.replace(new RegExp(`\\${fileEntryExt}$`), '') + : fileEntry; + return [importFile, newFileEntry]; +}; + export const create = context => { - const { options: [moduleUtilOptions] } = context; + const { + options: [moduleUtilOptions], + } = context; const packages = getPackages(process.cwd()); return moduleVisitor(node => { const { name, path: internalPath } = tryParse(node.value); - if (internalPath && packages.find(pkg => pkg.package.name === name)) { - context.report({ - node, - message: `Import for monorepo package '${name}' is internal.`, + const matchedPackage = packages.find(pkg => pkg.package.name === name); + const packageRoot = matchedPackage.location; + + // Need to take care of "files" field, since they are + // supposed to be part of the public API of the package + const absolutePathsForFiles = + matchedPackage.package.files && + matchedPackage.package.files.map(file => { + const fileOrDirOrGlob = path.join(packageRoot, file); + + try { + if (fs.lstatSync(fileOrDirOrGlob).isDirectory()) { + return path.join(fileOrDirOrGlob, '**', '*'); + } + return fileOrDirOrGlob; + } catch (e) { + return fileOrDirOrGlob; + } }); + const absoluteInternalPath = path.join(packageRoot, internalPath); + + if (!internalPath) return; + if (!matchedPackage) return; + if (absolutePathsForFiles) { + const isImportWithinFiles = absolutePathsForFiles.some(maybeGlob => { + // If import doesn't have an extension, strip it from the file entry + const [theImport, theFileEntry] = withoutExtension( + absoluteInternalPath, + maybeGlob + ); + return minimatch(theImport, theFileEntry); + }); + + if (isImportWithinFiles) return; } + + context.report({ + node, + message: `Import for monorepo package '${name}' is internal.`, + }); }, moduleUtilOptions); }; diff --git a/src/rules/no-relative-import.js b/src/rules/no-relative-import.js index 60553d2..c588f8f 100644 --- a/src/rules/no-relative-import.js +++ b/src/rules/no-relative-import.js @@ -13,7 +13,9 @@ export const meta = { }; export const create = context => { - const { options: [moduleUtilOptions] } = context; + const { + options: [moduleUtilOptions], + } = context; const sourceFsPath = context.getFilename(); const packages = getPackages(process.cwd()); @@ -33,9 +35,7 @@ export const create = context => { const subPackagePath = path.relative(pkg.location, resolvedPath); context.report({ node, - message: `Import for monorepo package '${ - pkg.package.name - }' should be absolute.`, + message: `Import for monorepo package '${pkg.package.name}' should be absolute.`, fix: fixer => { fixer.replaceText( node, diff --git a/test/fixture/yarn/packages/bar/allowed.js b/test/fixture/yarn/packages/bar/allowed.js new file mode 100644 index 0000000..e69de29 diff --git a/test/fixture/yarn/packages/bar/nested/.gitkeep b/test/fixture/yarn/packages/bar/nested/.gitkeep new file mode 100644 index 0000000..e69de29 diff --git a/test/fixture/yarn/packages/bar/package.json b/test/fixture/yarn/packages/bar/package.json index 9e4847f..56f2c91 100644 --- a/test/fixture/yarn/packages/bar/package.json +++ b/test/fixture/yarn/packages/bar/package.json @@ -1,4 +1,10 @@ { "name": "bar", - "version": "0.0.0" + "version": "0.0.0", + "files": [ + "index.js", + "allowed.js", + "nested", + "complex/glob/**/*.{js,ts}" + ] } diff --git a/test/rules/no-internal-import.test.js b/test/rules/no-internal-import.test.js index 6c70577..4a8bdab 100644 --- a/test/rules/no-internal-import.test.js +++ b/test/rules/no-internal-import.test.js @@ -3,7 +3,10 @@ import path from 'path'; import * as monorepo from '../../src'; const RULE = 'no-internal-import'; -const ERROR = { message: `Import for monorepo package 'foo' is internal.` }; +const ERROR = packageName => ({ + message: `Import for monorepo package '${packageName}' is internal.`, +}); + const fixtures = path.join(__dirname, '../fixture/yarn'); process.cwd = jest.fn(() => fixtures); @@ -18,13 +21,39 @@ ruleTester.run(RULE, monorepo.rules[RULE], { code: `import { pkg } from 'bar'`, filename: path.join(fixtures, 'packages/bar/index.js'), }, + { + code: `import { pkg } from 'bar/allowed'`, + filename: path.join(fixtures, 'packages/bar/index.js'), + }, + { + code: `import { pkg } from 'bar/nested/allowedToo'`, + filename: path.join(fixtures, 'packages/bar/index.js'), + }, + { + code: `import { pkg } from 'bar/nested/this/is/also/allowed'`, + filename: path.join(fixtures, 'packages/bar/index.js'), + }, + { + code: `import { pkg } from 'bar/complex/glob/anything/goes/here/test.ts'`, + filename: path.join(fixtures, 'packages/bar/index.js'), + }, ], invalid: [ { code: `import pkg from 'foo/src/pkg'`, filename: path.join(fixtures, 'packages/bar/index.js'), - errors: [ERROR], + errors: [ERROR('foo')], + }, + { + code: `import { pkg } from 'bar/allowedNot'`, + filename: path.join(fixtures, 'packages/bar/index.js'), + errors: [ERROR('bar')], + }, + { + code: `import { pkg } from 'bar/complex/glob/anything/goes/here/test.tsx'`, + filename: path.join(fixtures, 'packages/bar/index.js'), + errors: [ERROR('bar')], }, ], }); diff --git a/yarn.lock b/yarn.lock index a9b11be..504a42e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1830,9 +1830,10 @@ get-caller-file@^1.0.1: version "1.0.2" resolved "https://registry.yarnpkg.com/get-caller-file/-/get-caller-file-1.0.2.tgz#f702e63127e7e231c160a80c1554acb70d5047e5" -get-monorepo-packages@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/get-monorepo-packages/-/get-monorepo-packages-1.0.0.tgz#7deec63b9b14fab11ba2bd09c020f54ad9a761ea" +get-monorepo-packages@^1.1.0: + version "1.2.0" + resolved "https://registry.yarnpkg.com/get-monorepo-packages/-/get-monorepo-packages-1.2.0.tgz#3eee88d30b11a5f65955dec6ae331958b2a168e4" + integrity sha512-aDP6tH+eM3EuVSp3YyCutOcFS4Y9AhRRH9FAd+cjtR/g63Hx+DCXdKoP1ViRPUJz5wm+BOEXB4FhoffGHxJ7jQ== dependencies: globby "^7.1.1" load-json-file "^4.0.0" @@ -3508,8 +3509,9 @@ preserve@^0.2.0: resolved "https://registry.yarnpkg.com/preserve/-/preserve-0.2.0.tgz#815ed1f6ebc65926f865b310c0713bcb3315ce4b" prettier@^1.9.2: - version "1.9.2" - resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.9.2.tgz#96bc2132f7a32338e6078aeb29727178c6335827" + version "1.19.1" + resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.19.1.tgz#f7d7f5ff8a9cd872a7be4ca142095956a60797cb" + integrity sha512-s7PoyDv/II1ObgQunCbB9PdLmUcBZcnWOcxDh7O0N/UwDEsHyqkW+Qh28jW+mVuCdx7gLB0BotYI1Y6uI9iyew== pretty-format@^21.2.1: version "21.2.1"