From 179d3015184ce0b0a2d9b9b1abc7a4f422256943 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 17 Jan 2023 22:55:41 +0100 Subject: [PATCH 01/25] added eslint-plugin Signed-off-by: Patrik Oldsberg --- .changeset/tough-jeans-camp.md | 5 + .github/vale/Vocab/Backstage/accept.txt | 3 +- packages/eslint-plugin/README.md | 40 ++++++ packages/eslint-plugin/index.js | 29 ++++ packages/eslint-plugin/package.json | 16 +++ .../rules/no-forbidden-package-imports.js | 124 ++++++++++++++++++ yarn.lock | 6 + 7 files changed, 222 insertions(+), 1 deletion(-) create mode 100644 .changeset/tough-jeans-camp.md create mode 100644 packages/eslint-plugin/README.md create mode 100644 packages/eslint-plugin/index.js create mode 100644 packages/eslint-plugin/package.json create mode 100644 packages/eslint-plugin/rules/no-forbidden-package-imports.js diff --git a/.changeset/tough-jeans-camp.md b/.changeset/tough-jeans-camp.md new file mode 100644 index 0000000000..b7c08593e1 --- /dev/null +++ b/.changeset/tough-jeans-camp.md @@ -0,0 +1,5 @@ +--- +'@backstage/eslint-plugin': minor +--- + +Added a new ESLint plugin with common rules for Backstage projects. diff --git a/.github/vale/Vocab/Backstage/accept.txt b/.github/vale/Vocab/Backstage/accept.txt index f24b701d36..9e7cf7aa98 100644 --- a/.github/vale/Vocab/Backstage/accept.txt +++ b/.github/vale/Vocab/Backstage/accept.txt @@ -336,6 +336,7 @@ subfolders subheader subheaders subkey +subpath subroutes subtree superfences @@ -418,4 +419,4 @@ Dominik Henneke Kuang Frontside -Kaswell \ No newline at end of file +Kaswell diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md new file mode 100644 index 0000000000..a063d801d3 --- /dev/null +++ b/packages/eslint-plugin/README.md @@ -0,0 +1,40 @@ +# @backstage/eslint-plugin + +A collection of ESLint rules useful to Backstage projects. + +## Usage + +This ESLint plugin is part of the default lint configuration provided by the [Backstage CLI](https://www.npmjs.com/package/@backstage/cli), so you generally do not need to install it manually. + +If you do wish to install this plugin manually, start by adding it as a development dependency to your project: + +```sh +yarn add --dev @backstage/eslint-plugin +``` + +Then add it to your ESLint configuration: + +```js +extends: [ + 'plugin:@backstage/recommended', +], +``` + +Alternatively, if you want to install in individual rules manually: + +```js +plugins: [ + '@backstage', +], +rules: { + '@backstage/no-forbidden-package-imports': 'error', +} +``` + +## Rules + +The following rules are provided by this plugin: + +| Rule | Description | +| ----------------------------------------- | ------------------------------------------------------------------------------- | +| `@backstage/no-forbidden-package-imports` | Disallow internal monorepo imports from package subpaths that are not exported. | diff --git a/packages/eslint-plugin/index.js b/packages/eslint-plugin/index.js new file mode 100644 index 0000000000..11d17b2b8f --- /dev/null +++ b/packages/eslint-plugin/index.js @@ -0,0 +1,29 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +module.exports = { + configs: { + recommended: { + plugins: ['@backstage'], + rules: { + '@backstage/no-forbidden-package-imports': 'error', + }, + }, + }, + rules: { + 'no-forbidden-package-imports': require('./rules/no-forbidden-package-imports'), + }, +}; diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json new file mode 100644 index 0000000000..9724cec32b --- /dev/null +++ b/packages/eslint-plugin/package.json @@ -0,0 +1,16 @@ +{ + "name": "@backstage/eslint-plugin", + "description": "Backstage ESLint plugin", + "version": "0.0.0", + "publishConfig": { + "access": "public" + }, + "homepage": "https://backstage.io", + "repository": { + "type": "git", + "url": "https://github.com/backstage/backstage", + "directory": "packages/eslint-plugin" + }, + "license": "Apache-2.0", + "main": "./index.js" +} diff --git a/packages/eslint-plugin/rules/no-forbidden-package-imports.js b/packages/eslint-plugin/rules/no-forbidden-package-imports.js new file mode 100644 index 0000000000..d5cce80368 --- /dev/null +++ b/packages/eslint-plugin/rules/no-forbidden-package-imports.js @@ -0,0 +1,124 @@ +'use strict'; +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// @ts-check + +// Note: this file is ES5, because we want to avoid having to transpile it for local development + +var fs = require('fs'); +var path = require('path'); +/** @type {import('@manypkg/get-packages')} */ +var manypkg = require('@manypkg/get-packages'); + +// Loads all packages in the monorepo once, and caches the result +var getPackageMap = (function () { + var map = undefined; + var lastLoadAt = undefined; + + /** @returns {Record} */ + return function (/** @type {string} */ dir) { + if (map) { + // Only cache for 5 seconds, to avoid the need to reload ESLint servers + if (Date.now() - lastLoadAt > 5000) { + map = undefined; + } else { + return map; + } + } + var packages = manypkg.getPackagesSync(dir).packages; + if (!packages) { + return undefined; + } + map = {}; + lastLoadAt = Date.now(); + packages.forEach(function (pkg) { + map[pkg.packageJson.name] = pkg; + }); + return map; + }; +})(); + +module.exports = { + meta: { + type: 'problem', + messages: { + forbidden: '{{packageName}} does not export {{subPath}}', + }, + }, + create: function (/** @type import('eslint').Rule.RuleContext */ context) { + var packageMap = getPackageMap(context.getCwd()); + if (!packageMap) { + return; + } + + function checkImport(node) { + if (!node.source) { + return; + } + var importPath = node.source.value; + if (importPath[0] === '.') { + return; + } + + var pathParts = importPath.split('/'); + + // Check for match with plain name, then namespaced name + var name = pathParts.shift(); + if (!packageMap[name]) { + name += '/' + pathParts.shift(); + } + if (!packageMap[name]) { + return; + } + + // Empty subpaths are always allowed + var subPath = pathParts.join('/'); + if (!subPath) { + return; + } + + var pkg = packageMap[name]; + + // If the import is listed in the package.json exports field, we allow it + var exp = pkg.packageJson.exports; + if (exp && (exp[subPath] || exp['./' + subPath])) { + return; + } + + // If there's a package.json in the target directory, we allow the import + if (fs.existsSync(path.resolve(pkg.dir, subPath, 'package.json'))) { + return; + } + + context.report({ + node: node, + messageId: 'forbidden', + data: { + packageName: pkg.packageJson.name || '', + subPath: subPath, + }, + }); + } + + return { + ImportDeclaration: checkImport, + ExportAllDeclaration: checkImport, + ExportNamedDeclaration: checkImport, + ImportExpression: checkImport, + }; + }, +}; diff --git a/yarn.lock b/yarn.lock index 3d809a40c8..ac5c6d0541 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4095,6 +4095,12 @@ __metadata: languageName: unknown linkType: soft +"@backstage/eslint-plugin@workspace:packages/eslint-plugin": + version: 0.0.0-use.local + resolution: "@backstage/eslint-plugin@workspace:packages/eslint-plugin" + languageName: unknown + linkType: soft + "@backstage/integration-aws-node@workspace:^, @backstage/integration-aws-node@workspace:packages/integration-aws-node": version: 0.0.0-use.local resolution: "@backstage/integration-aws-node@workspace:packages/integration-aws-node" From 98b2bd1b51688ae5947a755dc13e023a6fa5e739 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 18 Jan 2023 21:53:55 +0100 Subject: [PATCH 02/25] eslint-plugin: added missing deps + lint config Signed-off-by: Patrik Oldsberg --- packages/eslint-plugin/.eslintrc.js | 35 +++++++++++++++++++++++++++++ packages/eslint-plugin/package.json | 11 ++++++++- yarn.lock | 3 +++ 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 packages/eslint-plugin/.eslintrc.js diff --git a/packages/eslint-plugin/.eslintrc.js b/packages/eslint-plugin/.eslintrc.js new file mode 100644 index 0000000000..aaf907cfbc --- /dev/null +++ b/packages/eslint-plugin/.eslintrc.js @@ -0,0 +1,35 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Custom lint config here since source is ES5 +module.exports = { + plugins: ['import'], + parserOptions: { + ecmaVersion: 5, + }, + rules: { + 'import/no-extraneous-dependencies': [ + 'error', + { + devDependencies: false, + optionalDependencies: true, + peerDependencies: true, + bundledDependencies: true, + }, + ], + 'no-unused-expressions': 'off', + }, +}; diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index 9724cec32b..ee5e62c41e 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -12,5 +12,14 @@ "directory": "packages/eslint-plugin" }, "license": "Apache-2.0", - "main": "./index.js" + "main": "./index.js", + "scripts": { + "lint": "backstage-cli package lint" + }, + "dependencies": { + "@manypkg/get-packages": "^1.1.3" + }, + "devDependencies": { + "@backstage/cli": "workspace:^" + } } diff --git a/yarn.lock b/yarn.lock index ac5c6d0541..06cd22d84a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4098,6 +4098,9 @@ __metadata: "@backstage/eslint-plugin@workspace:packages/eslint-plugin": version: 0.0.0-use.local resolution: "@backstage/eslint-plugin@workspace:packages/eslint-plugin" + dependencies: + "@backstage/cli": "workspace:^" + "@manypkg/get-packages": ^1.1.3 languageName: unknown linkType: soft From 4d05d5bf8362ac5e86c6e75918aa7cdbb14a3a79 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 11:10:38 +0100 Subject: [PATCH 03/25] eslint-plugin: refactor to ES2021 and add no-undeclared-imports rule Signed-off-by: Patrik Oldsberg --- packages/eslint-plugin/.eslintrc.js | 7 +- packages/eslint-plugin/index.js | 2 + packages/eslint-plugin/lib/getPackages.js | 63 +++++++ packages/eslint-plugin/lib/visitImports.js | 100 +++++++++++ packages/eslint-plugin/package.json | 3 +- .../rules/no-forbidden-package-imports.js | 91 ++-------- .../rules/no-undeclared-imports.js | 156 ++++++++++++++++++ yarn.lock | 1 + 8 files changed, 339 insertions(+), 84 deletions(-) create mode 100644 packages/eslint-plugin/lib/getPackages.js create mode 100644 packages/eslint-plugin/lib/visitImports.js create mode 100644 packages/eslint-plugin/rules/no-undeclared-imports.js diff --git a/packages/eslint-plugin/.eslintrc.js b/packages/eslint-plugin/.eslintrc.js index aaf907cfbc..74f93a6475 100644 --- a/packages/eslint-plugin/.eslintrc.js +++ b/packages/eslint-plugin/.eslintrc.js @@ -17,8 +17,9 @@ // Custom lint config here since source is ES5 module.exports = { plugins: ['import'], - parserOptions: { - ecmaVersion: 5, + env: { + node: true, + es2021: true, }, rules: { 'import/no-extraneous-dependencies': [ @@ -30,6 +31,6 @@ module.exports = { bundledDependencies: true, }, ], - 'no-unused-expressions': 'off', + // 'no-unused-expressions': 'off', }, }; diff --git a/packages/eslint-plugin/index.js b/packages/eslint-plugin/index.js index 11d17b2b8f..cf1dd20ab7 100644 --- a/packages/eslint-plugin/index.js +++ b/packages/eslint-plugin/index.js @@ -20,10 +20,12 @@ module.exports = { plugins: ['@backstage'], rules: { '@backstage/no-forbidden-package-imports': 'error', + '@backstage/no-undeclared-imports': 'error', }, }, }, rules: { 'no-forbidden-package-imports': require('./rules/no-forbidden-package-imports'), + 'no-undeclared-imports': require('./rules/no-undeclared-imports'), }, }; diff --git a/packages/eslint-plugin/lib/getPackages.js b/packages/eslint-plugin/lib/getPackages.js new file mode 100644 index 0000000000..91043fe0d5 --- /dev/null +++ b/packages/eslint-plugin/lib/getPackages.js @@ -0,0 +1,63 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// @ts-check + +/** @type {import('@manypkg/get-packages')} */ +const manypkg = require('@manypkg/get-packages'); + +/** + * @typedef ExtendedPackage + * @type {import('@manypkg/get-packages').Package & { packageJson: { exports?: Record }}} packageJson + */ + +/** + * @typedef PackageMap + * @type object + * + * @property {ExtendedPackage} root + * @property {ExtendedPackage[]} list + * @property {Map} map + */ + +// Loads all packages in the monorepo once, and caches the result +module.exports = (function () { + let result = undefined; + let lastLoadAt = undefined; + + /** @returns {PackageMap | undefined} */ + return function getPackages(/** @type {string} */ dir) { + if (result) { + // Only cache for 5 seconds, to avoid the need to reload ESLint servers + if (Date.now() - lastLoadAt > 5000) { + result = undefined; + } else { + return result; + } + } + const packages = manypkg.getPackagesSync(dir); + if (!packages) { + return undefined; + } + result = { + map: new Map(packages.packages.map(pkg => [pkg.packageJson.name, pkg])), + list: packages.packages, + root: packages.root, + }; + lastLoadAt = Date.now(); + return result; + }; +})(); diff --git a/packages/eslint-plugin/lib/visitImports.js b/packages/eslint-plugin/lib/visitImports.js new file mode 100644 index 0000000000..d73c3203a8 --- /dev/null +++ b/packages/eslint-plugin/lib/visitImports.js @@ -0,0 +1,100 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// @ts-check + +const getPackages = require('./getPackages'); + +/** + * @typedef LocalImport + * @type {object} + * @property {'local'} type + * @property {string} path + */ + +/** + * @typedef InternalImport + * @type {object} + * @property {'internal'} type + * @property {string} path + * @property {import('./getPackages').ExtendedPackage} package + */ + +/** + * @typedef ExternalImport + * @type {object} + * @property {'external'} type + * @property {string} path + * @property {string} packageName + */ + +/** + * @callback ImportVisitor + * @param {import('eslint').Rule.Node} node + * @param {LocalImport | InternalImport | ExternalImport} import + */ + +/** + * @param visitor - Visitor callback + * @param {import('eslint').Rule.RuleContext} context + * @param {ImportVisitor} visitor + */ +module.exports = function visitImports(context, visitor) { + const packages = getPackages(context.getCwd()); + if (!packages) { + return; + } + + function visit(node) { + if (!node.source) { + return; + } + const importPath = node.source.value; + if (importPath[0] === '.') { + return visitor(node, { type: 'local', path: importPath }); + } + + const pathParts = importPath.split('/'); + + // Check for match with plain name, then namespaced name + let packageName; + let subPath; + if (importPath[0] === '@') { + packageName = pathParts.slice(0, 2).join('/'); + subPath = pathParts.slice(2).join('/'); + } else { + packageName = pathParts[0]; + subPath = pathParts.slice(1).join('/'); + } + const pkg = packages?.map.get(packageName); + if (!pkg) { + return visitor(node, { + type: 'external', + path: subPath, + packageName: packageName, + }); + } + + return visitor(node, { type: 'internal', path: subPath, package: pkg }); + } + + return { + ImportDeclaration: visit, + ExportAllDeclaration: visit, + ExportNamedDeclaration: visit, + ImportExpression: visit, + }; +}; diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index ee5e62c41e..73862d2295 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -17,7 +17,8 @@ "lint": "backstage-cli package lint" }, "dependencies": { - "@manypkg/get-packages": "^1.1.3" + "@manypkg/get-packages": "^1.1.3", + "minimatch": "^5.1.2" }, "devDependencies": { "@backstage/cli": "workspace:^" diff --git a/packages/eslint-plugin/rules/no-forbidden-package-imports.js b/packages/eslint-plugin/rules/no-forbidden-package-imports.js index d5cce80368..bbf342dba0 100644 --- a/packages/eslint-plugin/rules/no-forbidden-package-imports.js +++ b/packages/eslint-plugin/rules/no-forbidden-package-imports.js @@ -1,4 +1,3 @@ -'use strict'; /* * Copyright 2023 The Backstage Authors * @@ -17,41 +16,9 @@ // @ts-check -// Note: this file is ES5, because we want to avoid having to transpile it for local development - -var fs = require('fs'); -var path = require('path'); -/** @type {import('@manypkg/get-packages')} */ -var manypkg = require('@manypkg/get-packages'); - -// Loads all packages in the monorepo once, and caches the result -var getPackageMap = (function () { - var map = undefined; - var lastLoadAt = undefined; - - /** @returns {Record} */ - return function (/** @type {string} */ dir) { - if (map) { - // Only cache for 5 seconds, to avoid the need to reload ESLint servers - if (Date.now() - lastLoadAt > 5000) { - map = undefined; - } else { - return map; - } - } - var packages = manypkg.getPackagesSync(dir).packages; - if (!packages) { - return undefined; - } - map = {}; - lastLoadAt = Date.now(); - packages.forEach(function (pkg) { - map[pkg.packageJson.name] = pkg; - }); - return map; - }; -})(); +const visitImports = require('../lib/visitImports'); +/** @type {import('eslint').Rule.RuleModule} */ module.exports = { meta: { type: 'problem', @@ -59,48 +26,19 @@ module.exports = { forbidden: '{{packageName}} does not export {{subPath}}', }, }, - create: function (/** @type import('eslint').Rule.RuleContext */ context) { - var packageMap = getPackageMap(context.getCwd()); - if (!packageMap) { - return; - } - - function checkImport(node) { - if (!node.source) { + create(context) { + return visitImports(context, (node, imp) => { + if (imp.type !== 'internal') { return; } - var importPath = node.source.value; - if (importPath[0] === '.') { - return; - } - - var pathParts = importPath.split('/'); - - // Check for match with plain name, then namespaced name - var name = pathParts.shift(); - if (!packageMap[name]) { - name += '/' + pathParts.shift(); - } - if (!packageMap[name]) { - return; - } - // Empty subpaths are always allowed - var subPath = pathParts.join('/'); - if (!subPath) { + if (!imp.path) { return; } - var pkg = packageMap[name]; - // If the import is listed in the package.json exports field, we allow it - var exp = pkg.packageJson.exports; - if (exp && (exp[subPath] || exp['./' + subPath])) { - return; - } - - // If there's a package.json in the target directory, we allow the import - if (fs.existsSync(path.resolve(pkg.dir, subPath, 'package.json'))) { + const exp = imp.package.packageJson.exports; + if (exp && (exp[imp.path] || exp['./' + imp.path])) { return; } @@ -108,17 +46,10 @@ module.exports = { node: node, messageId: 'forbidden', data: { - packageName: pkg.packageJson.name || '', - subPath: subPath, + packageName: imp.package.packageJson.name || imp.package.dir, + subPath: imp.path, }, }); - } - - return { - ImportDeclaration: checkImport, - ExportAllDeclaration: checkImport, - ExportNamedDeclaration: checkImport, - ImportExpression: checkImport, - }; + }); }, }; diff --git a/packages/eslint-plugin/rules/no-undeclared-imports.js b/packages/eslint-plugin/rules/no-undeclared-imports.js new file mode 100644 index 0000000000..462b4b2e31 --- /dev/null +++ b/packages/eslint-plugin/rules/no-undeclared-imports.js @@ -0,0 +1,156 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// @ts-check + +const path = require('path'); +const getPackageMap = require('../lib/getPackages'); +const visitImports = require('../lib/visitImports'); +const minimatch = require('minimatch'); + +const depFields = { + dep: 'dependencies', + dev: 'devDependencies', + peer: 'peerDependencies', +}; + +const peerDeps = ['react', 'react-dom', 'react-router', 'react-router-dom']; +const devModulePatterns = [ + new minimatch.Minimatch('!src/**'), + new minimatch.Minimatch('src/**/*.test.*'), + new minimatch.Minimatch('src/**/*.stories.*'), + new minimatch.Minimatch('src/**/__testUtils__/**'), + new minimatch.Minimatch('src/**/__mocks__/**'), + new minimatch.Minimatch('src/setupTests.*'), +]; + +function getExpectedDepType( + /** @type {string} */ impPath, + /** @type {string} */ modulePath, +) { + if (peerDeps.includes(impPath)) { + return 'peer'; + } + for (const pattern of devModulePatterns) { + if (pattern.match(modulePath)) { + return 'dev'; + } + } + return 'dep'; +} + +/** + * + * @param {import('@manypkg/get-packages').Package['packageJson']} pkg + * @param {string} name + * @param {ReturnType} expectedType + * @returns {{oldDepsField?: string, depsField: string} | undefined} + */ +function findConflict(pkg, name, expectedType) { + const isDep = pkg.dependencies?.[name]; + const isDevDep = pkg.devDependencies?.[name]; + const isPeerDep = pkg.peerDependencies?.[name]; + const depsField = depFields[expectedType]; + + if (expectedType === 'dep' && !isDep) { + const oldDepsField = isDevDep ? depFields.dev : undefined; + return { oldDepsField, depsField }; + } else if (expectedType === 'dev' && !isDevDep && !isDep && !isPeerDep) { + return { oldDepsField: undefined, depsField }; + } else if (expectedType === 'peer' && !isPeerDep) { + const oldDepsField = isDep + ? depFields.dep + : isDevDep + ? depFields.dev + : undefined; + + return { oldDepsField, depsField }; + } +} + +function getAddFlagForConflict(conflict) { + switch (conflict?.depsField) { + case depFields.dep: + return ''; + case depFields.dev: + return ' --dev'; + case depFields.peer: + return ' --peer'; + default: + return ''; + } +} + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + meta: { + type: 'problem', + messages: { + undeclared: + "{{ packageName }} must be declared in {{ depsField }} of {{ packageJsonPath }}, run 'yarn --cwd {{ packagePath }} add{{ addFlag }} {{ packageName }}' from the project root.", + switch: + '{{ packageName }} is declared in {{ oldDepsField }}, but should be moved to {{ depsField }} in {{ packageJsonPath }}.', + }, + }, + create(context) { + const packages = getPackageMap(context.getCwd()); + if (!packages) { + return {}; + } + const filePath = context.getPhysicalFilename + ? context.getPhysicalFilename() + : context.getFilename(); + + const localPkg = packages.list.find(p => + filePath.startsWith(p.dir + path.sep), + ); + if (!localPkg) { + return {}; + } + + return visitImports(context, (node, imp) => { + if (imp.type !== 'external') { + return; + } + + const modulePath = path.relative(localPkg.dir, filePath); + const expectedType = getExpectedDepType(imp.packageName, modulePath); + + const conflict = findConflict( + localPkg.packageJson, + imp.packageName, + expectedType, + ); + + if (conflict) { + const packagePath = path.relative(packages.root.dir, localPkg.dir); + const packageJsonPath = path.join(packagePath, 'package.json'); + + context.report({ + node, + messageId: conflict.oldDepsField ? 'switch' : 'undeclared', + data: { + ...conflict, + packagePath, + addFlag: getAddFlagForConflict(conflict), + packageName: imp.packageName, + packageJsonPath: packageJsonPath, + }, + }); + } + }); + }, +}; diff --git a/yarn.lock b/yarn.lock index 06cd22d84a..e0e1da349c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4101,6 +4101,7 @@ __metadata: dependencies: "@backstage/cli": "workspace:^" "@manypkg/get-packages": ^1.1.3 + minimatch: ^5.1.2 languageName: unknown linkType: soft From f459b168f03033ca6c4025ce16941d7bc0480835 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 14:19:19 +0100 Subject: [PATCH 04/25] eslint-plugin: add support for detecting Node.js builtins Signed-off-by: Patrik Oldsberg --- packages/eslint-plugin/lib/visitImports.js | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/lib/visitImports.js b/packages/eslint-plugin/lib/visitImports.js index d73c3203a8..c89d65fb49 100644 --- a/packages/eslint-plugin/lib/visitImports.js +++ b/packages/eslint-plugin/lib/visitImports.js @@ -16,6 +16,7 @@ // @ts-check +const { builtinModules } = require('module'); const getPackages = require('./getPackages'); /** @@ -41,10 +42,18 @@ const getPackages = require('./getPackages'); * @property {string} packageName */ +/** + * @typedef BuiltinImport + * @type {object} + * @property {'builtin'} type + * @property {string} path + * @property {string} packageName + */ + /** * @callback ImportVisitor * @param {import('eslint').Rule.Node} node - * @param {LocalImport | InternalImport | ExternalImport} import + * @param {LocalImport | InternalImport | ExternalImport | BuiltinImport} import */ /** @@ -81,10 +90,17 @@ module.exports = function visitImports(context, visitor) { } const pkg = packages?.map.get(packageName); if (!pkg) { + if ( + packageName.startsWith('node:') || + builtinModules.includes(packageName) + ) { + return visitor(node, { type: 'builtin', path: subPath, packageName }); + } + return visitor(node, { type: 'external', path: subPath, - packageName: packageName, + packageName, }); } From 9e192d7f4a67e5a1c1ad35b2b6c27e2207b6c079 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 14:19:41 +0100 Subject: [PATCH 05/25] eslint-plugin: dogfood lint config Signed-off-by: Patrik Oldsberg --- packages/eslint-plugin/.eslintrc.js | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/packages/eslint-plugin/.eslintrc.js b/packages/eslint-plugin/.eslintrc.js index 74f93a6475..7fc68c3c5d 100644 --- a/packages/eslint-plugin/.eslintrc.js +++ b/packages/eslint-plugin/.eslintrc.js @@ -16,21 +16,13 @@ // Custom lint config here since source is ES5 module.exports = { - plugins: ['import'], + plugins: ['@backstage'], env: { node: true, es2021: true, }, rules: { - 'import/no-extraneous-dependencies': [ - 'error', - { - devDependencies: false, - optionalDependencies: true, - peerDependencies: true, - bundledDependencies: true, - }, - ], - // 'no-unused-expressions': 'off', + '@backstage/no-undeclared-imports': ['error'], + 'no-unused-expressions': 'off', }, }; From 8b4a758ed28e1d1268894f6d71d0847e365e286f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 14:38:40 +0100 Subject: [PATCH 06/25] eslint-plugin: add support for require calls Signed-off-by: Patrik Oldsberg --- packages/eslint-plugin/lib/visitImports.js | 36 ++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/lib/visitImports.js b/packages/eslint-plugin/lib/visitImports.js index c89d65fb49..5184fb8d79 100644 --- a/packages/eslint-plugin/lib/visitImports.js +++ b/packages/eslint-plugin/lib/visitImports.js @@ -56,6 +56,36 @@ const getPackages = require('./getPackages'); * @param {LocalImport | InternalImport | ExternalImport | BuiltinImport} import */ +/** + * @param {import('estree').ImportDeclaration | import('estree').ExportAllDeclaration | import('estree').ExportNamedDeclaration | import('estree').ImportExpression | import('estree').SimpleCallExpression} node + * @returns {string | undefined} + */ +function getImportPath(node) { + /** @type {import('estree').Expression | import('estree').SpreadElement | undefined | null} */ + let pathNode; + + if (node.type === 'CallExpression') { + if ( + node.callee.type === 'Identifier' && + node.callee.name == 'require' && + node.arguments.length === 1 + ) { + pathNode = node.arguments[0]; + } + } else { + pathNode = node.source; + } + + if (pathNode?.type !== 'Literal') { + return undefined; + } + if (typeof pathNode.value !== 'string') { + return undefined; + } + + return pathNode.value; +} + /** * @param visitor - Visitor callback * @param {import('eslint').Rule.RuleContext} context @@ -68,10 +98,11 @@ module.exports = function visitImports(context, visitor) { } function visit(node) { - if (!node.source) { + const importPath = getImportPath(node); + if (!importPath) { return; } - const importPath = node.source.value; + if (importPath[0] === '.') { return visitor(node, { type: 'local', path: importPath }); } @@ -112,5 +143,6 @@ module.exports = function visitImports(context, visitor) { ExportAllDeclaration: visit, ExportNamedDeclaration: visit, ImportExpression: visit, + CallExpression: visit, }; }; From 765b7ee355ddcbb01c57942796a7cc57952c7839 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 16:14:07 +0100 Subject: [PATCH 07/25] eslint-plugin: backwards compatiblity check for packages without exports Signed-off-by: Patrik Oldsberg --- packages/eslint-plugin/lib/getPackages.js | 2 +- .../rules/no-forbidden-package-imports.js | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/lib/getPackages.js b/packages/eslint-plugin/lib/getPackages.js index 91043fe0d5..318d0654c2 100644 --- a/packages/eslint-plugin/lib/getPackages.js +++ b/packages/eslint-plugin/lib/getPackages.js @@ -21,7 +21,7 @@ const manypkg = require('@manypkg/get-packages'); /** * @typedef ExtendedPackage - * @type {import('@manypkg/get-packages').Package & { packageJson: { exports?: Record }}} packageJson + * @type {import('@manypkg/get-packages').Package & { packageJson: { exports?: Record, files?: Array }}} packageJson */ /** diff --git a/packages/eslint-plugin/rules/no-forbidden-package-imports.js b/packages/eslint-plugin/rules/no-forbidden-package-imports.js index bbf342dba0..6c861e17f5 100644 --- a/packages/eslint-plugin/rules/no-forbidden-package-imports.js +++ b/packages/eslint-plugin/rules/no-forbidden-package-imports.js @@ -41,6 +41,20 @@ module.exports = { if (exp && (exp[imp.path] || exp['./' + imp.path])) { return; } + if (!exp) { + // If there's no exports field, we allow anything listed in files, except dist + const files = imp.package.packageJson.files; + if ( + !files || + files.some(f => !f.startsWith('dist') && imp.path.startsWith(f)) + ) { + return; + } + // And also package.json + if (imp.path === 'package.json') { + return; + } + } context.report({ node: node, From d5eee2944cdc4ccb83f643ad22197c8064e28959 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 16:15:17 +0100 Subject: [PATCH 08/25] eslint-plugin: improved peer deps check and consider package role Signed-off-by: Patrik Oldsberg --- .../rules/no-undeclared-imports.js | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/rules/no-undeclared-imports.js b/packages/eslint-plugin/rules/no-undeclared-imports.js index 462b4b2e31..cec3503f8f 100644 --- a/packages/eslint-plugin/rules/no-undeclared-imports.js +++ b/packages/eslint-plugin/rules/no-undeclared-imports.js @@ -27,7 +27,6 @@ const depFields = { peer: 'peerDependencies', }; -const peerDeps = ['react', 'react-dom', 'react-router', 'react-router-dom']; const devModulePatterns = [ new minimatch.Minimatch('!src/**'), new minimatch.Minimatch('src/**/*.test.*'), @@ -38,12 +37,34 @@ const devModulePatterns = [ ]; function getExpectedDepType( + localPkg, /** @type {string} */ impPath, /** @type {string} */ modulePath, ) { - if (peerDeps.includes(impPath)) { - return 'peer'; + const role = localPkg?.backstage?.role; + // Some package roles have known dependency types + switch (role) { + case 'common-library': + case 'web-library': + case 'frontend-plugin': + case 'frontend-plugin-module': + case 'node-library': + case 'backend-plugin': + case 'backend-plugin-module': + switch (impPath) { + case 'react': + case 'react-dom': + case 'react-router': + case 'react-router-dom': + return 'peer'; + } + case 'cli': + case 'frontend': + case 'backend': + default: + break; } + for (const pattern of devModulePatterns) { if (pattern.match(modulePath)) { return 'dev'; @@ -65,7 +86,7 @@ function findConflict(pkg, name, expectedType) { const isPeerDep = pkg.peerDependencies?.[name]; const depsField = depFields[expectedType]; - if (expectedType === 'dep' && !isDep) { + if (expectedType === 'dep' && !isDep && !isPeerDep) { const oldDepsField = isDevDep ? depFields.dev : undefined; return { oldDepsField, depsField }; } else if (expectedType === 'dev' && !isDevDep && !isDep && !isPeerDep) { @@ -127,7 +148,11 @@ module.exports = { } const modulePath = path.relative(localPkg.dir, filePath); - const expectedType = getExpectedDepType(imp.packageName, modulePath); + const expectedType = getExpectedDepType( + localPkg, + imp.packageName, + modulePath, + ); const conflict = findConflict( localPkg.packageJson, From 58e481a4f9a4889d11720b18727d96eb2406b749 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 16:15:48 +0100 Subject: [PATCH 09/25] eslint-plugin: ignore type imports when checking deps Signed-off-by: Patrik Oldsberg --- packages/eslint-plugin/lib/visitImports.js | 39 +++++++++++++------ .../rules/no-undeclared-imports.js | 16 ++++++++ 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/packages/eslint-plugin/lib/visitImports.js b/packages/eslint-plugin/lib/visitImports.js index 5184fb8d79..ed16c26c7f 100644 --- a/packages/eslint-plugin/lib/visitImports.js +++ b/packages/eslint-plugin/lib/visitImports.js @@ -23,6 +23,7 @@ const getPackages = require('./getPackages'); * @typedef LocalImport * @type {object} * @property {'local'} type + * @property {'value' | 'type'} kind * @property {string} path */ @@ -30,6 +31,7 @@ const getPackages = require('./getPackages'); * @typedef InternalImport * @type {object} * @property {'internal'} type + * @property {'value' | 'type'} kind * @property {string} path * @property {import('./getPackages').ExtendedPackage} package */ @@ -38,6 +40,7 @@ const getPackages = require('./getPackages'); * @typedef ExternalImport * @type {object} * @property {'external'} type + * @property {'value' | 'type'} kind * @property {string} path * @property {string} packageName */ @@ -46,6 +49,7 @@ const getPackages = require('./getPackages'); * @typedef BuiltinImport * @type {object} * @property {'builtin'} type + * @property {'value' | 'type'} kind * @property {string} path * @property {string} packageName */ @@ -58,9 +62,9 @@ const getPackages = require('./getPackages'); /** * @param {import('estree').ImportDeclaration | import('estree').ExportAllDeclaration | import('estree').ExportNamedDeclaration | import('estree').ImportExpression | import('estree').SimpleCallExpression} node - * @returns {string | undefined} + * @returns {undefined | {path: string, kind: 'type' | 'value'}} */ -function getImportPath(node) { +function getImportInfo(node) { /** @type {import('estree').Expression | import('estree').SpreadElement | undefined | null} */ let pathNode; @@ -83,7 +87,9 @@ function getImportPath(node) { return undefined; } - return pathNode.value; + /** @type {any} */ + const anyNode = node; + return { path: pathNode.value, kind: anyNode.importKind ?? 'value' }; } /** @@ -98,21 +104,21 @@ module.exports = function visitImports(context, visitor) { } function visit(node) { - const importPath = getImportPath(node); - if (!importPath) { + const info = getImportInfo(node); + if (!info) { return; } - if (importPath[0] === '.') { - return visitor(node, { type: 'local', path: importPath }); + if (info.path[0] === '.') { + return visitor(node, { type: 'local', ...info }); } - const pathParts = importPath.split('/'); + const pathParts = info.path.split('/'); // Check for match with plain name, then namespaced name let packageName; let subPath; - if (importPath[0] === '@') { + if (info.path[0] === '@') { packageName = pathParts.slice(0, 2).join('/'); subPath = pathParts.slice(2).join('/'); } else { @@ -125,17 +131,28 @@ module.exports = function visitImports(context, visitor) { packageName.startsWith('node:') || builtinModules.includes(packageName) ) { - return visitor(node, { type: 'builtin', path: subPath, packageName }); + return visitor(node, { + type: 'builtin', + kind: info.kind, + path: subPath, + packageName, + }); } return visitor(node, { type: 'external', + kind: info.kind, path: subPath, packageName, }); } - return visitor(node, { type: 'internal', path: subPath, package: pkg }); + return visitor(node, { + type: 'internal', + kind: info.kind, + path: subPath, + package: pkg, + }); } return { diff --git a/packages/eslint-plugin/rules/no-undeclared-imports.js b/packages/eslint-plugin/rules/no-undeclared-imports.js index cec3503f8f..f7f7b17354 100644 --- a/packages/eslint-plugin/rules/no-undeclared-imports.js +++ b/packages/eslint-plugin/rules/no-undeclared-imports.js @@ -146,6 +146,10 @@ module.exports = { if (imp.type !== 'external') { return; } + // We leave checking of type imports to the repo-tools check + if (imp.kind === 'type') { + return; + } const modulePath = path.relative(localPkg.dir, filePath); const expectedType = getExpectedDepType( @@ -161,6 +165,18 @@ module.exports = { ); if (conflict) { + try { + const fullImport = imp.path + ? `${imp.packageName}/${imp.path}` + : imp.packageName; + require.resolve(fullImport, { + paths: [localPkg.dir], + }); + } catch { + // If the dependency doesn't resolve then it's likely a type import, ignore + return; + } + const packagePath = path.relative(packages.root.dir, localPkg.dir); const packageJsonPath = path.join(packagePath, 'package.json'); From 11df6f42b9c0b8ddb5408b1fb5b5196558f1c413 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 16:16:41 +0100 Subject: [PATCH 10/25] cli: use new eslint plugin + fix issues Signed-off-by: Patrik Oldsberg --- cypress/.eslintrc.js | 9 ------ packages/app/cypress/.eslintrc.json | 9 ------ .../backend-defaults/src/CreateBackend.ts | 2 +- packages/cli/asset-types/asset-types.d.ts | 2 +- packages/cli/config/eslint-factory.js | 31 ++----------------- .../lib/builder/buildTypeDefinitionsWorker.ts | 2 +- packages/cli/src/lib/version.test.ts | 1 - packages/codemods/.eslintrc.js | 15 +-------- packages/codemods/package.json | 2 +- .../LogViewer/RealLogViewer.test.tsx | 2 +- .../LogViewer/useLogViewerSelection.test.tsx | 2 +- packages/create-app/bin/backstage-create-app | 2 +- .../packages/app/cypress/.eslintrc.json | 9 ------ packages/e2e-test/.eslintrc.js | 15 +-------- packages/e2e-test/package.json | 16 +++++----- packages/e2e-test/src/commands/run.ts | 2 +- .../src/commands/type-deps/type-deps.ts | 2 +- .../cypress/.eslintrc.json | 9 ------ packages/techdocs-cli/cypress/.eslintrc.json | 9 ------ .../src/run.ts | 2 +- plugins/code-climate/dev/index.tsx | 2 +- plugins/code-climate/src/setupTests.ts | 2 +- .../example/templates/CostInsightsClient.ts | 2 +- plugins/example-todo-list/.eslintrc.js | 15 +-------- .../scripts/build-nunjucks.js | 2 +- .../src/components/TriggerDialog/testUtils.ts | 2 +- scripts/assemble-manifest.js | 2 +- scripts/check-docs-quality.js | 2 +- scripts/check-if-release.js | 2 +- scripts/create-github-release.js | 2 +- scripts/create-release-tag.js | 2 +- scripts/list-deprecations.js | 2 +- scripts/prepare-release.js | 2 +- scripts/snyk-github-issue-sync.ts | 2 +- scripts/verify-api-reference.js | 2 +- scripts/verify-changesets.js | 2 +- scripts/verify-links.js | 2 +- scripts/verify-lockfile-duplicates.js | 2 +- 38 files changed, 42 insertions(+), 150 deletions(-) diff --git a/cypress/.eslintrc.js b/cypress/.eslintrc.js index a77de4a629..07f264452d 100644 --- a/cypress/.eslintrc.js +++ b/cypress/.eslintrc.js @@ -2,15 +2,6 @@ module.exports = { extends: [require.resolve('@backstage/cli/config/eslint.backend')], rules: { 'no-console': 0, - 'import/no-extraneous-dependencies': [ - 'error', - { - devDependencies: true, - optionalDependencies: false, - peerDependencies: false, - bundledDependencies: false, - }, - ], 'jest/valid-expect': 'off', 'jest/expect-expect': 'off', 'no-restricted-syntax': 'off', diff --git a/packages/app/cypress/.eslintrc.json b/packages/app/cypress/.eslintrc.json index 2b3a458b95..a467608916 100644 --- a/packages/app/cypress/.eslintrc.json +++ b/packages/app/cypress/.eslintrc.json @@ -7,15 +7,6 @@ { "assertFunctionNames": ["expect", "cy.contains"] } - ], - "import/no-extraneous-dependencies": [ - "error", - { - "devDependencies": true, - "optionalDependencies": true, - "peerDependencies": true, - "bundledDependencies": true - } ] } } diff --git a/packages/backend-defaults/src/CreateBackend.ts b/packages/backend-defaults/src/CreateBackend.ts index e6b5a15336..5cbed2c3df 100644 --- a/packages/backend-defaults/src/CreateBackend.ts +++ b/packages/backend-defaults/src/CreateBackend.ts @@ -40,7 +40,7 @@ import { } from '@backstage/backend-plugin-api'; // Internal import of the type to avoid needing to export this. -// eslint-disable-next-line monorepo/no-internal-import +// eslint-disable-next-line @backstage/no-forbidden-package-imports import type { InternalSharedBackendEnvironment } from '@backstage/backend-plugin-api/src/wiring/createSharedEnvironment'; export const defaultServiceFactories = [ diff --git a/packages/cli/asset-types/asset-types.d.ts b/packages/cli/asset-types/asset-types.d.ts index 0bd0922004..9bd158bb8e 100644 --- a/packages/cli/asset-types/asset-types.d.ts +++ b/packages/cli/asset-types/asset-types.d.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -/* eslint-disable import/no-extraneous-dependencies */ +/* eslint-disable @backstage/no-undeclared-imports */ /// /// diff --git a/packages/cli/config/eslint-factory.js b/packages/cli/config/eslint-factory.js index a40aa79fb3..f344af4ab0 100644 --- a/packages/cli/config/eslint-factory.js +++ b/packages/cli/config/eslint-factory.js @@ -60,11 +60,11 @@ function createConfig(dir, extraConfig = {}) { '@spotify/eslint-config-typescript', 'prettier', 'plugin:jest/recommended', - 'plugin:monorepo/recommended', + 'plugin:@backstage/recommended', ...(extraExtends ?? []), ], parser: '@typescript-eslint/parser', - plugins: ['import', ...(plugins ?? [])], + plugins: ['import', 'monorepo', ...(plugins ?? [])], env: { jest: true, ...env, @@ -87,33 +87,8 @@ function createConfig(dir, extraConfig = {}) { '@typescript-eslint/no-shadow': 'error', '@typescript-eslint/no-redeclare': 'error', 'no-undef': 'off', + 'monorepo/no-relative-import': 'error', 'import/newline-after-import': 'error', - 'import/no-extraneous-dependencies': [ - 'error', - { - devDependencies: dir - ? [ - `!${joinPath(dir, 'src/**')}`, - joinPath(dir, 'src/**/*.test.*'), - joinPath(dir, 'src/**/*.stories.*'), - joinPath(dir, 'src/**/__testUtils__/**'), - joinPath(dir, 'src/**/__mocks__/**'), - joinPath(dir, 'src/setupTests.*'), - ] - : [ - // Legacy config for packages that don't provide a dir - '**/*.test.*', - '**/*.stories.*', - '**/__testUtils__/**', - '**/__mocks__/**', - '**/src/setupTests.*', - '**/dev/**', - ], - optionalDependencies: true, - peerDependencies: true, - bundledDependencies: true, - }, - ], 'no-unused-expressions': 'off', '@typescript-eslint/no-unused-expressions': 'error', '@typescript-eslint/consistent-type-assertions': 'error', diff --git a/packages/cli/src/lib/builder/buildTypeDefinitionsWorker.ts b/packages/cli/src/lib/builder/buildTypeDefinitionsWorker.ts index 2533131a40..cf37c2a664 100644 --- a/packages/cli/src/lib/builder/buildTypeDefinitionsWorker.ts +++ b/packages/cli/src/lib/builder/buildTypeDefinitionsWorker.ts @@ -50,7 +50,7 @@ export async function buildTypeDefinitionsWorker( */ const { PackageJsonLookup, - // eslint-disable-next-line import/no-extraneous-dependencies + // eslint-disable-next-line @backstage/no-undeclared-imports } = require('@rushstack/node-core-library/lib/PackageJsonLookup'); const old = PackageJsonLookup.prototype.tryGetPackageJsonFilePathFor; diff --git a/packages/cli/src/lib/version.test.ts b/packages/cli/src/lib/version.test.ts index fb9bf83e7c..604129679b 100644 --- a/packages/cli/src/lib/version.test.ts +++ b/packages/cli/src/lib/version.test.ts @@ -17,7 +17,6 @@ import mockFs from 'mock-fs'; import { packageVersions, createPackageVersionProvider } from './version'; import { Lockfile } from './versioning'; -// eslint-disable-next-line monorepo/no-internal-import import corePluginApiPkg from '@backstage/core-plugin-api/package.json'; describe('createPackageVersionProvider', () => { diff --git a/packages/codemods/.eslintrc.js b/packages/codemods/.eslintrc.js index f9fb2e27c9..e2a53a6ad2 100644 --- a/packages/codemods/.eslintrc.js +++ b/packages/codemods/.eslintrc.js @@ -1,14 +1 @@ -module.exports = require('@backstage/cli/config/eslint-factory')(__dirname, { - rules: { - 'no-console': 0, - 'import/no-extraneous-dependencies': [ - 'error', - { - devDependencies: true, - optionalDependencies: false, - peerDependencies: false, - bundledDependencies: false, - }, - ], - }, -}); +module.exports = require('@backstage/cli/config/eslint-factory')(__dirname); diff --git a/packages/codemods/package.json b/packages/codemods/package.json index cbcc0613d4..1ae4d04485 100644 --- a/packages/codemods/package.json +++ b/packages/codemods/package.json @@ -35,6 +35,7 @@ "dependencies": { "@backstage/cli-common": "workspace:^", "chalk": "^4.0.0", + "commander": "^9.1.0", "jscodeshift": "^0.14.0", "jscodeshift-add-imports": "^1.0.10" }, @@ -42,7 +43,6 @@ "@backstage/cli": "workspace:^", "@types/jscodeshift": "^0.11.0", "@types/node": "^16.11.26", - "commander": "^9.1.0", "ts-node": "^10.0.0" }, "nodemonConfig": { diff --git a/packages/core-components/src/components/LogViewer/RealLogViewer.test.tsx b/packages/core-components/src/components/LogViewer/RealLogViewer.test.tsx index dd707778e1..e1aac76381 100644 --- a/packages/core-components/src/components/LogViewer/RealLogViewer.test.tsx +++ b/packages/core-components/src/components/LogViewer/RealLogViewer.test.tsx @@ -18,7 +18,7 @@ import React, { ReactNode } from 'react'; import userEvent from '@testing-library/user-event'; import { renderInTestApp } from '@backstage/test-utils'; import { RealLogViewer } from './RealLogViewer'; -// eslint-disable-next-line import/no-extraneous-dependencies +// eslint-disable-next-line @backstage/no-undeclared-imports import copyToClipboard from 'copy-to-clipboard'; // Used by useCopyToClipboard diff --git a/packages/core-components/src/components/LogViewer/useLogViewerSelection.test.tsx b/packages/core-components/src/components/LogViewer/useLogViewerSelection.test.tsx index 7e00d50f33..93a6dfd7e9 100644 --- a/packages/core-components/src/components/LogViewer/useLogViewerSelection.test.tsx +++ b/packages/core-components/src/components/LogViewer/useLogViewerSelection.test.tsx @@ -20,7 +20,7 @@ import { TestApiProvider, MockErrorApi } from '@backstage/test-utils'; import { errorApiRef } from '@backstage/core-plugin-api'; import { AnsiLine } from './AnsiProcessor'; import { useLogViewerSelection } from './useLogViewerSelection'; -// eslint-disable-next-line import/no-extraneous-dependencies +// eslint-disable-next-line @backstage/no-undeclared-imports import copyToClipboard from 'copy-to-clipboard'; // Used by useCopyToClipboard diff --git a/packages/create-app/bin/backstage-create-app b/packages/create-app/bin/backstage-create-app index 60dfd4990a..4639ea96dc 100755 --- a/packages/create-app/bin/backstage-create-app +++ b/packages/create-app/bin/backstage-create-app @@ -25,7 +25,7 @@ if (!isLocal || process.env.BACKSTAGE_E2E_CLI_TEST) { require('..'); } else { // Only used for development, so should be a devDependency - // eslint-disable-next-line import/no-extraneous-dependencies + // eslint-disable-next-line @backstage/no-undeclared-imports require('ts-node').register({ transpileOnly: true, project: path.resolve(__dirname, '../../../tsconfig.json'), diff --git a/packages/create-app/templates/default-app/packages/app/cypress/.eslintrc.json b/packages/create-app/templates/default-app/packages/app/cypress/.eslintrc.json index 2b3a458b95..a467608916 100644 --- a/packages/create-app/templates/default-app/packages/app/cypress/.eslintrc.json +++ b/packages/create-app/templates/default-app/packages/app/cypress/.eslintrc.json @@ -7,15 +7,6 @@ { "assertFunctionNames": ["expect", "cy.contains"] } - ], - "import/no-extraneous-dependencies": [ - "error", - { - "devDependencies": true, - "optionalDependencies": true, - "peerDependencies": true, - "bundledDependencies": true - } ] } } diff --git a/packages/e2e-test/.eslintrc.js b/packages/e2e-test/.eslintrc.js index f9fb2e27c9..e2a53a6ad2 100644 --- a/packages/e2e-test/.eslintrc.js +++ b/packages/e2e-test/.eslintrc.js @@ -1,14 +1 @@ -module.exports = require('@backstage/cli/config/eslint-factory')(__dirname, { - rules: { - 'no-console': 0, - 'import/no-extraneous-dependencies': [ - 'error', - { - devDependencies: true, - optionalDependencies: false, - peerDependencies: false, - bundledDependencies: false, - }, - ], - }, -}); +module.exports = require('@backstage/cli/config/eslint-factory')(__dirname); diff --git a/packages/e2e-test/package.json b/packages/e2e-test/package.json index 374eaa6d18..ee447a1736 100644 --- a/packages/e2e-test/package.json +++ b/packages/e2e-test/package.json @@ -26,22 +26,24 @@ "clean": "backstage-cli package clean" }, "bin": "bin/e2e-test", - "devDependencies": { - "@backstage/cli": "workspace:^", + "dependencies": { "@backstage/cli-common": "workspace:^", "@backstage/errors": "workspace:^", - "@types/fs-extra": "^9.0.1", - "@types/node": "^16.11.26", - "@types/puppeteer": "^5.4.4", "chalk": "^4.0.0", "commander": "^9.1.0", "cross-fetch": "^3.1.5", "fs-extra": "10.1.0", "handlebars": "^4.7.3", - "nodemon": "^2.0.2", "pgtools": "^0.3.0", "puppeteer": "^17.0.0", - "tree-kill": "^1.2.2", + "tree-kill": "^1.2.2" + }, + "devDependencies": { + "@backstage/cli": "workspace:^", + "@types/fs-extra": "^9.0.1", + "@types/node": "^16.11.26", + "@types/puppeteer": "^5.4.4", + "nodemon": "^2.0.2", "ts-node": "^10.0.0" }, "nodemonConfig": { diff --git a/packages/e2e-test/src/commands/run.ts b/packages/e2e-test/src/commands/run.ts index 3eedb90b21..254198ebb4 100644 --- a/packages/e2e-test/src/commands/run.ts +++ b/packages/e2e-test/src/commands/run.ts @@ -159,7 +159,7 @@ async function buildDistWorkspace(workspaceName: string, rootDir: string) { appendDeps(pkg); } - // eslint-disable-next-line import/no-extraneous-dependencies + // eslint-disable-next-line @backstage/no-forbidden-package-imports appendDeps(require('@backstage/create-app/package.json')); print(`Preparing workspace`); diff --git a/packages/repo-tools/src/commands/type-deps/type-deps.ts b/packages/repo-tools/src/commands/type-deps/type-deps.ts index d4dd28aa14..4f8e5e5529 100644 --- a/packages/repo-tools/src/commands/type-deps/type-deps.ts +++ b/packages/repo-tools/src/commands/type-deps/type-deps.ts @@ -17,7 +17,7 @@ import fs from 'fs'; import { resolve as resolvePath } from 'path'; // Cba polluting root package.json, we'll have this -// eslint-disable-next-line import/no-extraneous-dependencies +// eslint-disable-next-line @backstage/no-undeclared-imports import chalk from 'chalk'; import { getPackages, Package } from '@manypkg/get-packages'; diff --git a/packages/techdocs-cli-embedded-app/cypress/.eslintrc.json b/packages/techdocs-cli-embedded-app/cypress/.eslintrc.json index 2b3a458b95..a467608916 100644 --- a/packages/techdocs-cli-embedded-app/cypress/.eslintrc.json +++ b/packages/techdocs-cli-embedded-app/cypress/.eslintrc.json @@ -7,15 +7,6 @@ { "assertFunctionNames": ["expect", "cy.contains"] } - ], - "import/no-extraneous-dependencies": [ - "error", - { - "devDependencies": true, - "optionalDependencies": true, - "peerDependencies": true, - "bundledDependencies": true - } ] } } diff --git a/packages/techdocs-cli/cypress/.eslintrc.json b/packages/techdocs-cli/cypress/.eslintrc.json index 2481ac5715..06965a82da 100644 --- a/packages/techdocs-cli/cypress/.eslintrc.json +++ b/packages/techdocs-cli/cypress/.eslintrc.json @@ -7,15 +7,6 @@ { "assertFunctionNames": ["expect", "cy.contains", "cy.document"] } - ], - "import/no-extraneous-dependencies": [ - "error", - { - "devDependencies": true, - "optionalDependencies": true, - "peerDependencies": true, - "bundledDependencies": true - } ] } } diff --git a/plugins/catalog-backend-module-incremental-ingestion/src/run.ts b/plugins/catalog-backend-module-incremental-ingestion/src/run.ts index 08d95f4417..2a58227862 100644 --- a/plugins/catalog-backend-module-incremental-ingestion/src/run.ts +++ b/plugins/catalog-backend-module-incremental-ingestion/src/run.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -// eslint-disable-next-line import/no-extraneous-dependencies +// eslint-disable-next-line @backstage/no-undeclared-imports import { databaseFactory, discoveryFactory, diff --git a/plugins/code-climate/dev/index.tsx b/plugins/code-climate/dev/index.tsx index d5fc4c28e3..060679337e 100644 --- a/plugins/code-climate/dev/index.tsx +++ b/plugins/code-climate/dev/index.tsx @@ -15,7 +15,7 @@ */ import { Entity } from '@backstage/catalog-model'; -// eslint-disable-next-line import/no-extraneous-dependencies +// eslint-disable-next-line @backstage/no-undeclared-imports import { createDevApp, EntityGridItem } from '@backstage/dev-utils'; import { Grid } from '@material-ui/core'; import React from 'react'; diff --git a/plugins/code-climate/src/setupTests.ts b/plugins/code-climate/src/setupTests.ts index 43d9e09713..eb4654b869 100644 --- a/plugins/code-climate/src/setupTests.ts +++ b/plugins/code-climate/src/setupTests.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -/* eslint-disable import/no-extraneous-dependencies */ +/* eslint-disable @backstage/no-undeclared-imports */ import '@testing-library/jest-dom'; import 'cross-fetch/polyfill'; diff --git a/plugins/cost-insights/src/example/templates/CostInsightsClient.ts b/plugins/cost-insights/src/example/templates/CostInsightsClient.ts index 50c3bb1470..046196f26b 100644 --- a/plugins/cost-insights/src/example/templates/CostInsightsClient.ts +++ b/plugins/cost-insights/src/example/templates/CostInsightsClient.ts @@ -22,7 +22,7 @@ // IMPORTANT: Remove the lines below to enable type checking and linting // @ts-nocheck -/* eslint-disable import/no-extraneous-dependencies */ +/* eslint-disable @backstage/no-undeclared-imports */ import { CostInsightsApi, diff --git a/plugins/example-todo-list/.eslintrc.js b/plugins/example-todo-list/.eslintrc.js index 8c18e76229..e2a53a6ad2 100644 --- a/plugins/example-todo-list/.eslintrc.js +++ b/plugins/example-todo-list/.eslintrc.js @@ -1,14 +1 @@ -module.exports = { - extends: [require.resolve('@backstage/cli/config/eslint')], - rules: { - 'import/no-extraneous-dependencies': [ - 'error', - { - devDependencies: true, - optionalDependencies: true, - peerDependencies: true, - bundledDependencies: true, - }, - ], - }, -}; +module.exports = require('@backstage/cli/config/eslint-factory')(__dirname); diff --git a/plugins/scaffolder-backend/scripts/build-nunjucks.js b/plugins/scaffolder-backend/scripts/build-nunjucks.js index 8b98ac4305..8183de77ee 100755 --- a/plugins/scaffolder-backend/scripts/build-nunjucks.js +++ b/plugins/scaffolder-backend/scripts/build-nunjucks.js @@ -16,7 +16,7 @@ */ /* eslint-disable no-restricted-syntax */ -/* eslint-disable import/no-extraneous-dependencies */ +/* eslint-disable @backstage/no-undeclared-imports */ const path = require('path'); diff --git a/plugins/splunk-on-call/src/components/TriggerDialog/testUtils.ts b/plugins/splunk-on-call/src/components/TriggerDialog/testUtils.ts index 764ac94e2c..9beb4e150e 100644 --- a/plugins/splunk-on-call/src/components/TriggerDialog/testUtils.ts +++ b/plugins/splunk-on-call/src/components/TriggerDialog/testUtils.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -// eslint-disable-next-line import/no-extraneous-dependencies +// eslint-disable-next-line @backstage/no-undeclared-imports import { act, fireEvent, diff --git a/scripts/assemble-manifest.js b/scripts/assemble-manifest.js index f68a83447d..f01bc7fd9b 100755 --- a/scripts/assemble-manifest.js +++ b/scripts/assemble-manifest.js @@ -1,5 +1,5 @@ #!/usr/bin/env node -/* eslint-disable import/no-extraneous-dependencies */ +/* eslint-disable @backstage/no-undeclared-imports */ /* * Copyright 2022 The Backstage Authors * diff --git a/scripts/check-docs-quality.js b/scripts/check-docs-quality.js index 10c4b99976..3dcbd6ef81 100755 --- a/scripts/check-docs-quality.js +++ b/scripts/check-docs-quality.js @@ -56,7 +56,7 @@ async function listFiles(dir = '') { // caused by the script. In CI, we want to ensure vale linter is run. async function exitIfMissingVale() { try { - // eslint-disable-next-line import/no-extraneous-dependencies + // eslint-disable-next-line @backstage/no-undeclared-imports await require('command-exists')('vale'); } catch (e) { if (process.env.CI) { diff --git a/scripts/check-if-release.js b/scripts/check-if-release.js index e7dabe14a4..c12b27b0ef 100755 --- a/scripts/check-if-release.js +++ b/scripts/check-if-release.js @@ -1,5 +1,5 @@ #!/usr/bin/env node -/* eslint-disable import/no-extraneous-dependencies */ +/* eslint-disable @backstage/no-undeclared-imports */ /* * Copyright 2020 The Backstage Authors * diff --git a/scripts/create-github-release.js b/scripts/create-github-release.js index 4b3891c5ce..b64d7a9cb5 100755 --- a/scripts/create-github-release.js +++ b/scripts/create-github-release.js @@ -1,5 +1,5 @@ #!/usr/bin/env node -/* eslint-disable import/no-extraneous-dependencies */ +/* eslint-disable @backstage/no-undeclared-imports */ /* * Copyright 2020 The Backstage Authors * diff --git a/scripts/create-release-tag.js b/scripts/create-release-tag.js index 28417024fd..311b46d4e5 100755 --- a/scripts/create-release-tag.js +++ b/scripts/create-release-tag.js @@ -1,5 +1,5 @@ #!/usr/bin/env node -/* eslint-disable import/no-extraneous-dependencies */ +/* eslint-disable @backstage/no-undeclared-imports */ /* * Copyright 2020 The Backstage Authors * diff --git a/scripts/list-deprecations.js b/scripts/list-deprecations.js index 8047158138..22aff96d67 100755 --- a/scripts/list-deprecations.js +++ b/scripts/list-deprecations.js @@ -15,7 +15,7 @@ * limitations under the License. */ -/* eslint-disable import/no-extraneous-dependencies */ +/* eslint-disable @backstage/no-undeclared-imports */ const _ = require('lodash'); const fs = require('fs-extra'); diff --git a/scripts/prepare-release.js b/scripts/prepare-release.js index 973f4fad58..4019a71cf3 100755 --- a/scripts/prepare-release.js +++ b/scripts/prepare-release.js @@ -1,5 +1,5 @@ #!/usr/bin/env node -/* eslint-disable import/no-extraneous-dependencies */ +/* eslint-disable @backstage/no-undeclared-imports */ /* * Copyright 2022 The Backstage Authors * diff --git a/scripts/snyk-github-issue-sync.ts b/scripts/snyk-github-issue-sync.ts index da64e2ea82..ea7d42a2a4 100755 --- a/scripts/snyk-github-issue-sync.ts +++ b/scripts/snyk-github-issue-sync.ts @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -/* eslint-disable import/no-extraneous-dependencies */ +/* eslint-disable @backstage/no-undeclared-imports */ import { Octokit } from '@octokit/rest'; import minimist from 'minimist'; // Generated by GitHub workflow .github/workflows/snyk-github-issue-creator diff --git a/scripts/verify-api-reference.js b/scripts/verify-api-reference.js index 84a526604d..091e803c33 100755 --- a/scripts/verify-api-reference.js +++ b/scripts/verify-api-reference.js @@ -15,7 +15,7 @@ * limitations under the License. */ -/* eslint-disable import/no-extraneous-dependencies */ +/* eslint-disable @backstage/no-undeclared-imports */ const { resolve: resolvePath } = require('path'); const { promises: fs } = require('fs'); diff --git a/scripts/verify-changesets.js b/scripts/verify-changesets.js index 9e3064eeac..ae3f07840a 100755 --- a/scripts/verify-changesets.js +++ b/scripts/verify-changesets.js @@ -15,7 +15,7 @@ * limitations under the License. */ -/* eslint-disable import/no-extraneous-dependencies */ +/* eslint-disable @backstage/no-undeclared-imports */ const { resolve: resolvePath } = require('path'); const fs = require('fs-extra'); diff --git a/scripts/verify-links.js b/scripts/verify-links.js index 05ad387c37..7664cadad5 100755 --- a/scripts/verify-links.js +++ b/scripts/verify-links.js @@ -15,7 +15,7 @@ * limitations under the License. */ -/* eslint-disable import/no-extraneous-dependencies */ +/* eslint-disable @backstage/no-undeclared-imports */ const { resolve: resolvePath, join: joinPath, dirname } = require('path'); const fs = require('fs').promises; diff --git a/scripts/verify-lockfile-duplicates.js b/scripts/verify-lockfile-duplicates.js index cc22664219..820fba2ea5 100644 --- a/scripts/verify-lockfile-duplicates.js +++ b/scripts/verify-lockfile-duplicates.js @@ -15,7 +15,7 @@ * limitations under the License. */ -/* eslint-disable import/no-extraneous-dependencies */ +/* eslint-disable @backstage/no-undeclared-imports */ const { execFile: execFileCb } = require('child_process'); const { resolve: resolvePath, dirname: dirnamePath } = require('path'); From 4753d26b949a521c48334882c6bac4eb6cd2a2c3 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 16:42:23 +0100 Subject: [PATCH 11/25] eslint-plugin: add no-relative-monorepo-imports rule Signed-off-by: Patrik Oldsberg --- packages/eslint-plugin/index.js | 2 + packages/eslint-plugin/lib/getPackages.js | 10 ++- .../rules/no-relative-monorepo-imports.js | 80 +++++++++++++++++++ .../rules/no-undeclared-imports.js | 4 +- 4 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 packages/eslint-plugin/rules/no-relative-monorepo-imports.js diff --git a/packages/eslint-plugin/index.js b/packages/eslint-plugin/index.js index cf1dd20ab7..20a0ff2d18 100644 --- a/packages/eslint-plugin/index.js +++ b/packages/eslint-plugin/index.js @@ -20,12 +20,14 @@ module.exports = { plugins: ['@backstage'], rules: { '@backstage/no-forbidden-package-imports': 'error', + '@backstage/no-relative-monorepo-imports': 'error', '@backstage/no-undeclared-imports': 'error', }, }, }, rules: { 'no-forbidden-package-imports': require('./rules/no-forbidden-package-imports'), + 'no-relative-monorepo-imports': require('./rules/no-relative-monorepo-imports'), 'no-undeclared-imports': require('./rules/no-undeclared-imports'), }, }; diff --git a/packages/eslint-plugin/lib/getPackages.js b/packages/eslint-plugin/lib/getPackages.js index 318d0654c2..6c8c1a66b2 100644 --- a/packages/eslint-plugin/lib/getPackages.js +++ b/packages/eslint-plugin/lib/getPackages.js @@ -16,7 +16,7 @@ // @ts-check -/** @type {import('@manypkg/get-packages')} */ +const path = require('path'); const manypkg = require('@manypkg/get-packages'); /** @@ -31,6 +31,7 @@ const manypkg = require('@manypkg/get-packages'); * @property {ExtendedPackage} root * @property {ExtendedPackage[]} list * @property {Map} map + * @property {(path: string) => ExtendedPackage | undefined} byPath */ // Loads all packages in the monorepo once, and caches the result @@ -56,6 +57,13 @@ module.exports = (function () { map: new Map(packages.packages.map(pkg => [pkg.packageJson.name, pkg])), list: packages.packages, root: packages.root, + byPath(filePath) { + return packages.packages.find(pkg => { + if (!path.relative(pkg.dir, filePath).startsWith('..')) { + return pkg; + } + }); + }, }; lastLoadAt = Date.now(); return result; diff --git a/packages/eslint-plugin/rules/no-relative-monorepo-imports.js b/packages/eslint-plugin/rules/no-relative-monorepo-imports.js new file mode 100644 index 0000000000..cb18eb506c --- /dev/null +++ b/packages/eslint-plugin/rules/no-relative-monorepo-imports.js @@ -0,0 +1,80 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// @ts-check + +const path = require('path'); +const visitImports = require('../lib/visitImports'); +const getPackageMap = require('../lib/getPackages'); + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + meta: { + type: 'problem', + messages: { + outside: 'Import of {{path}} is outside of any known monorepo package', + forbidden: + "Relative imports of monorepo packages are forbidden, use '{{newImport}}' instead", + }, + }, + create(context) { + const packages = getPackageMap(context.getCwd()); + if (!packages) { + return {}; + } + const filePath = context.getPhysicalFilename + ? context.getPhysicalFilename() + : context.getFilename(); + + const localPkg = packages.byPath(filePath); + if (!localPkg) { + return {}; + } + + return visitImports(context, (node, imp) => { + if (imp.type !== 'local') { + return; + } + + const target = path.resolve(path.dirname(filePath), imp.path); + if (!path.relative(localPkg.dir, target).startsWith('..')) { + return; + } + + const targetPkg = packages.byPath(target); + if (!targetPkg) { + context.report({ + node: node, + messageId: 'outside', + data: { + path: target, + }, + }); + return; + } + + const targetPath = path.relative(targetPkg.dir, target); + const targetName = targetPkg.packageJson.name ?? ''; + context.report({ + node: node, + messageId: 'forbidden', + data: { + newImport: targetPath ? `${targetName}/${targetPath}` : targetName, + }, + }); + }); + }, +}; diff --git a/packages/eslint-plugin/rules/no-undeclared-imports.js b/packages/eslint-plugin/rules/no-undeclared-imports.js index f7f7b17354..4a8756b71e 100644 --- a/packages/eslint-plugin/rules/no-undeclared-imports.js +++ b/packages/eslint-plugin/rules/no-undeclared-imports.js @@ -135,9 +135,7 @@ module.exports = { ? context.getPhysicalFilename() : context.getFilename(); - const localPkg = packages.list.find(p => - filePath.startsWith(p.dir + path.sep), - ); + const localPkg = packages.byPath(filePath); if (!localPkg) { return {}; } From 906af5db533ee9356b61a7f3844339d3d77cbbb4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 16:48:43 +0100 Subject: [PATCH 12/25] cli: remove unnecessary lint rules and fix issues Signed-off-by: Patrik Oldsberg --- packages/cli/config/eslint-factory.js | 8 +------- packages/cli/package.json | 1 - packages/cli/src/lib/version.ts | 2 +- packages/core-plugin-api/src/app/types.ts | 2 +- packages/create-app/src/index.ts | 1 + packages/create-app/src/lib/versions.ts | 2 +- 6 files changed, 5 insertions(+), 11 deletions(-) diff --git a/packages/cli/config/eslint-factory.js b/packages/cli/config/eslint-factory.js index f344af4ab0..dde853fdff 100644 --- a/packages/cli/config/eslint-factory.js +++ b/packages/cli/config/eslint-factory.js @@ -64,7 +64,7 @@ function createConfig(dir, extraConfig = {}) { ...(extraExtends ?? []), ], parser: '@typescript-eslint/parser', - plugins: ['import', 'monorepo', ...(plugins ?? [])], + plugins: ['import', ...(plugins ?? [])], env: { jest: true, ...env, @@ -87,7 +87,6 @@ function createConfig(dir, extraConfig = {}) { '@typescript-eslint/no-shadow': 'error', '@typescript-eslint/no-redeclare': 'error', 'no-undef': 'off', - 'monorepo/no-relative-import': 'error', 'import/newline-after-import': 'error', 'no-unused-expressions': 'off', '@typescript-eslint/no-unused-expressions': 'error', @@ -110,9 +109,6 @@ function createConfig(dir, extraConfig = {}) { ...(restrictedSrcImports ?? []), ], patterns: [ - // Avoid cross-package imports - '**/../../**/*/src/**', - '**/../../**/*/src', // Prevent imports of stories or tests '*.stories*', '*.test*', @@ -162,8 +158,6 @@ function createConfig(dir, extraConfig = {}) { ...(restrictedImports ?? []), ...(restrictedTestImports ?? []), ], - // Avoid cross-package imports - patterns: ['**/../../**/*/src/**', '**/../../**/*/src'], }, ], }, diff --git a/packages/cli/package.json b/packages/cli/package.json index 909ac8460c..359f16adfa 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -76,7 +76,6 @@ "eslint-plugin-import": "^2.25.4", "eslint-plugin-jest": "^27.0.0", "eslint-plugin-jsx-a11y": "^6.5.1", - "eslint-plugin-monorepo": "^0.3.2", "eslint-plugin-react": "^7.28.0", "eslint-plugin-react-hooks": "^4.3.0", "eslint-webpack-plugin": "^3.1.1", diff --git a/packages/cli/src/lib/version.ts b/packages/cli/src/lib/version.ts index 5a199236f5..bf89842d5f 100644 --- a/packages/cli/src/lib/version.ts +++ b/packages/cli/src/lib/version.ts @@ -19,7 +19,7 @@ import semver from 'semver'; import { paths } from './paths'; import { Lockfile } from './versioning'; -/* eslint-disable monorepo/no-relative-import */ +/* eslint-disable @backstage/no-relative-monorepo-imports */ /* This is a list of all packages used by the templates. If dependencies are added or removed, diff --git a/packages/core-plugin-api/src/app/types.ts b/packages/core-plugin-api/src/app/types.ts index 2a71eda1f4..f3e0547ca4 100644 --- a/packages/core-plugin-api/src/app/types.ts +++ b/packages/core-plugin-api/src/app/types.ts @@ -17,7 +17,7 @@ // This is a bit of a hack that we use to avoid having to redeclare these types // within this package or have an explicit dependency on core-app-api. // These types end up being inlined and duplicated into this package at build time. -// eslint-disable-next-line no-restricted-imports +// eslint-disable-next-line @backstage/no-relative-monorepo-imports export type { BootErrorPageProps, SignInPageProps, diff --git a/packages/create-app/src/index.ts b/packages/create-app/src/index.ts index 216fab6740..fa235e6175 100644 --- a/packages/create-app/src/index.ts +++ b/packages/create-app/src/index.ts @@ -22,6 +22,7 @@ import { program } from 'commander'; import { exitWithError } from './lib/errors'; +// eslint-disable-next-line @backstage/no-relative-monorepo-imports import { version } from '../../../package.json'; import createApp from './createApp'; diff --git a/packages/create-app/src/lib/versions.ts b/packages/create-app/src/lib/versions.ts index b89b1d5463..72f5c2c9d5 100644 --- a/packages/create-app/src/lib/versions.ts +++ b/packages/create-app/src/lib/versions.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -/* eslint-disable monorepo/no-relative-import */ +/* eslint-disable @backstage/no-relative-monorepo-imports */ /* This is a list of all packages used by the template. If dependencies are added or removed, From ef73e8626bf906febc682f1f31451a4668e329c3 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 17:04:24 +0100 Subject: [PATCH 13/25] eslint-plugin: add docs for no-undeclared-imports Signed-off-by: Patrik Oldsberg --- .../docs/rules/no-undeclared-imports.md | 84 +++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-undeclared-imports.md diff --git a/packages/eslint-plugin/docs/rules/no-undeclared-imports.md b/packages/eslint-plugin/docs/rules/no-undeclared-imports.md new file mode 100644 index 0000000000..6102c772e6 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-undeclared-imports.md @@ -0,0 +1,84 @@ +# @backstage/no-undeclared-imports + +Forbid imports of external packages that have not been declared in the appropriate dependencies field in `package.json`. + +## Usage + +Add the rules as follows, it has no options: + +```js +"@backstage/no-undeclared-imports": ["error"] +``` + +The following patterns are considered files used during development, and only need dependencies to be declared in devDependencies: + +```python +!src/** # Any files outside of src are considered dev files +src/**/*.test.* +src/**/*.stories.* +src/**/__testUtils__/** +src/**/__mocks__/** +src/setupTests.* +``` + +## Rule Details + +Given the following `package.json`: + +```json +{ + "name": "@backstage/plugin-foo", + "backstage": { + "role": "frontend-plugin" + }, + "dependencies": { + "react": "^17.0.0" + }, + "devDependencies": { + "@backstage/core-plugin-api": "^1.0.0" + }, + "peerDependencies": { + "@backstage/config": "*" + } +} +``` + +### Fail + +Inside `src/my-plugin.ts`: + +```ts +// Should be declared as a dependency +const _ = require('lodash'); +import _ from 'lodash'; + +// React should be a peer dependency in frontend plugins +import react from 'react'; + +// Should be declared as a dependency, not a dev dependency +import { useApi } from '@backstage/core-plugin-api'; +``` + +Inside `src/my-plugin.test.ts` (a test file): + +```ts +// Should be declared as a dev dependency +const _ = require('lodash'); +import _ from 'lodash'; +``` + +### Pass + +Inside `src/my-plugin.ts`: + +```ts +// Declared in peerDependencies, so it is allowed +import { ConfigReader } from '@backstage/config'; +``` + +Inside `src/my-plugin.test.ts` (a test file): + +```ts +// Declared as a dev dependency inside a test file +import { useApi } from '@backstage/core-plugin-api'; +``` From 5105b9848a6ed57512697f3c80d198dcdabfdbae Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 17:10:51 +0100 Subject: [PATCH 14/25] eslint-plugin: add docs for no-relative-monorepo-imports Signed-off-by: Patrik Oldsberg --- .../rules/no-relative-monorepo-imports.md | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-relative-monorepo-imports.md diff --git a/packages/eslint-plugin/docs/rules/no-relative-monorepo-imports.md b/packages/eslint-plugin/docs/rules/no-relative-monorepo-imports.md new file mode 100644 index 0000000000..6a41a73fb6 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-relative-monorepo-imports.md @@ -0,0 +1,43 @@ +# @backstage/no-relative-monorepo-imports + +Forbid relative imports that reach outside of the package in a monorepo. + +## Usage + +Add the rules as follows, it has no options: + +```js +"@backstage/no-relative-monorepo-imports": ["error"] +``` + +The following patterns are considered files used during development, and only need dependencies to be declared in devDependencies: + +```python +!src/** # Any files outside of src are considered dev files +src/**/*.test.* +src/**/*.stories.* +src/**/__testUtils__/** +src/**/__mocks__/** +src/setupTests.* +``` + +## Rule Details + +Assuming an import from for example `plugins/bar/src/index.ts`: + +### Fail + +```ts +import { FooCard } from '../../foo'; + +import { FooCard } from '../../foo/src/components/FooCard'; +``` + +### Pass + +```ts +import { FooCard } from '@internal/plugin-foo'; + +// This is allowed by this rule, but not by no-forbidden-package-imports +import { FooCard } from '@internal/plugin-foo/src/components/FooCard'; +``` From 015c0e0f89025c7ad8925a2bf7ad5321efaf4adf Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 17:15:55 +0100 Subject: [PATCH 15/25] eslint-plugin: add docs for no-forbidden-package-imports Signed-off-by: Patrik Oldsberg --- .../rules/no-forbidden-package-imports.md | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-forbidden-package-imports.md diff --git a/packages/eslint-plugin/docs/rules/no-forbidden-package-imports.md b/packages/eslint-plugin/docs/rules/no-forbidden-package-imports.md new file mode 100644 index 0000000000..7a6ae11c37 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-forbidden-package-imports.md @@ -0,0 +1,52 @@ +# @backstage/no-forbidden-package-imports + +Disallow internal monorepo imports from package subpaths that are not exported. + +## Usage + +Add the rules as follows, it has no options: + +```js +"@backstage/no-forbidden-package-imports": ["error"] +``` + +## Rule Details + +Given the following two target packages: + +```json +{ + "name": "@backstage/plugin-foo", + "files": ["dist", "type-utils"] +} +``` + +```json +{ + "name": "@backstage/plugin-bar", + "exports": { + ".": "./src/index.ts", + "./testUtils": "./src/testUtils/index.ts", + "./package.json": "./package.json" + } +} +``` + +### Fail + +```ts +import { FooCard } from '@backstage/plugin-foo/src/components'; +import { BarCard } from '@backstage/plugin-bar/src/components'; +``` + +### Pass + +```ts +import { FooCard } from '@backstage/plugin-foo'; +import { FooType } from '@backstage/plugin-foo/type-utils'; +import fooPkg from '@backstage/plugin-foo/package.json'; + +import { BarCard } from '@backstage/plugin-bar'; +import { renderBarCardExtension } from '@backstage/plugin-bar/testUtils'; +import barPkg from '@backstage/plugin-bar/package.json'; +``` From 71bf2eb0ed63c7fc3482d3d043e055ac865cb418 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 17:20:24 +0100 Subject: [PATCH 16/25] eslint-plugin: add doc rule links and update README Signed-off-by: Patrik Oldsberg --- packages/eslint-plugin/README.md | 8 +++++--- .../eslint-plugin/rules/no-forbidden-package-imports.js | 5 +++++ .../eslint-plugin/rules/no-relative-monorepo-imports.js | 5 +++++ packages/eslint-plugin/rules/no-undeclared-imports.js | 5 +++++ 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index a063d801d3..c311670603 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -35,6 +35,8 @@ rules: { The following rules are provided by this plugin: -| Rule | Description | -| ----------------------------------------- | ------------------------------------------------------------------------------- | -| `@backstage/no-forbidden-package-imports` | Disallow internal monorepo imports from package subpaths that are not exported. | +| Rule | Description | +| --------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------ | +| [@backstage/no-forbidden-package-imports](./docs/rules/no-forbidden-package-imports.md) | Disallow internal monorepo imports from package subpaths that are not exported. | +| [@backstage/no-relative-monorepo-imports](./docs/rules/no-relative-monorepo-imports.md) | Forbid relative imports that reach outside of the package in a monorepo. | +| [@backstage/no-undeclared-imports](./docs/rules/no-undeclared-imports.md) | Forbid imports of external packages that have not been declared in the appropriate dependencies field in `package.json`. | diff --git a/packages/eslint-plugin/rules/no-forbidden-package-imports.js b/packages/eslint-plugin/rules/no-forbidden-package-imports.js index 6c861e17f5..e7c1acca27 100644 --- a/packages/eslint-plugin/rules/no-forbidden-package-imports.js +++ b/packages/eslint-plugin/rules/no-forbidden-package-imports.js @@ -25,6 +25,11 @@ module.exports = { messages: { forbidden: '{{packageName}} does not export {{subPath}}', }, + docs: { + description: + 'Disallow internal monorepo imports from package subpaths that are not exported.', + url: 'https://github.com/backstage/backstage/blob/master/packages/eslint-plugin/docs/rules/no-forbidden-package-imports.md', + }, }, create(context) { return visitImports(context, (node, imp) => { diff --git a/packages/eslint-plugin/rules/no-relative-monorepo-imports.js b/packages/eslint-plugin/rules/no-relative-monorepo-imports.js index cb18eb506c..df30d35f67 100644 --- a/packages/eslint-plugin/rules/no-relative-monorepo-imports.js +++ b/packages/eslint-plugin/rules/no-relative-monorepo-imports.js @@ -29,6 +29,11 @@ module.exports = { forbidden: "Relative imports of monorepo packages are forbidden, use '{{newImport}}' instead", }, + docs: { + description: + 'Forbid relative imports that reach outside of the package in a monorepo.', + url: 'https://github.com/backstage/backstage/blob/master/packages/eslint-plugin/docs/rules/no-relative-monorepo-imports.md', + }, }, create(context) { const packages = getPackageMap(context.getCwd()); diff --git a/packages/eslint-plugin/rules/no-undeclared-imports.js b/packages/eslint-plugin/rules/no-undeclared-imports.js index 4a8756b71e..4f60b712d9 100644 --- a/packages/eslint-plugin/rules/no-undeclared-imports.js +++ b/packages/eslint-plugin/rules/no-undeclared-imports.js @@ -125,6 +125,11 @@ module.exports = { switch: '{{ packageName }} is declared in {{ oldDepsField }}, but should be moved to {{ depsField }} in {{ packageJsonPath }}.', }, + docs: { + description: + 'Forbid imports of external packages that have not been declared in the appropriate dependencies field in `package.json`.', + url: 'https://github.com/backstage/backstage/blob/master/packages/eslint-plugin/docs/rules/no-undeclared-imports.md', + }, }, create(context) { const packages = getPackageMap(context.getCwd()); From dd8a9afe668ab0bb69bc2d65130e513ce58e8426 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 17:24:49 +0100 Subject: [PATCH 17/25] added and updated changesets for eslint-plugin addition Signed-off-by: Patrik Oldsberg --- .changeset/few-moons-accept.md | 5 +++++ .changeset/tough-jeans-camp.md | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changeset/few-moons-accept.md diff --git a/.changeset/few-moons-accept.md b/.changeset/few-moons-accept.md new file mode 100644 index 0000000000..c790e837c8 --- /dev/null +++ b/.changeset/few-moons-accept.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +Replaced several monorepo lint rules with new rules from `@backstage/eslint-plugin`. See the [README](https://github.com/import-js/eslint-plugin-import/blob/main/packages/eslint-plugin/README.md) for a full list of rules. diff --git a/.changeset/tough-jeans-camp.md b/.changeset/tough-jeans-camp.md index b7c08593e1..8c7a53190f 100644 --- a/.changeset/tough-jeans-camp.md +++ b/.changeset/tough-jeans-camp.md @@ -2,4 +2,4 @@ '@backstage/eslint-plugin': minor --- -Added a new ESLint plugin with common rules for Backstage projects. +Added a new ESLint plugin with common rules for Backstage projects. See the [README](https://github.com/import-js/eslint-plugin-import/blob/main/packages/eslint-plugin/README.md) for more details. From aefe3a562495c2a1e8efbb926958654d85f0e4fb Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 3 Feb 2023 18:06:52 +0100 Subject: [PATCH 18/25] eslint-plugin: added tests for no-relative-monorepo-imports Signed-off-by: Patrik Oldsberg --- packages/eslint-plugin/.eslintrc.js | 3 + packages/eslint-plugin/package.json | 6 +- .../src/__fixtures__/monorepo/package.json | 6 + .../monorepo/packages/bar/package.json | 9 ++ .../monorepo/packages/foo/package.json | 7 + .../src/no-relative-monorepo-imports.test.ts | 68 +++++++++ yarn.lock | 140 +----------------- 7 files changed, 102 insertions(+), 137 deletions(-) create mode 100644 packages/eslint-plugin/src/__fixtures__/monorepo/package.json create mode 100644 packages/eslint-plugin/src/__fixtures__/monorepo/packages/bar/package.json create mode 100644 packages/eslint-plugin/src/__fixtures__/monorepo/packages/foo/package.json create mode 100644 packages/eslint-plugin/src/no-relative-monorepo-imports.test.ts diff --git a/packages/eslint-plugin/.eslintrc.js b/packages/eslint-plugin/.eslintrc.js index 7fc68c3c5d..01d1b181d7 100644 --- a/packages/eslint-plugin/.eslintrc.js +++ b/packages/eslint-plugin/.eslintrc.js @@ -21,6 +21,9 @@ module.exports = { node: true, es2021: true, }, + parserOptions: { + sourceType: 'module', + }, rules: { '@backstage/no-undeclared-imports': ['error'], 'no-unused-expressions': 'off', diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index 73862d2295..84fc16b98f 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -14,13 +14,15 @@ "license": "Apache-2.0", "main": "./index.js", "scripts": { - "lint": "backstage-cli package lint" + "lint": "backstage-cli package lint", + "test": "backstage-cli package test" }, "dependencies": { "@manypkg/get-packages": "^1.1.3", "minimatch": "^5.1.2" }, "devDependencies": { - "@backstage/cli": "workspace:^" + "@backstage/cli": "workspace:^", + "eslint": "^8.33.0" } } diff --git a/packages/eslint-plugin/src/__fixtures__/monorepo/package.json b/packages/eslint-plugin/src/__fixtures__/monorepo/package.json new file mode 100644 index 0000000000..c35178c46f --- /dev/null +++ b/packages/eslint-plugin/src/__fixtures__/monorepo/package.json @@ -0,0 +1,6 @@ +{ + "name": "root", + "workspaces": [ + "packages/*" + ] +} diff --git a/packages/eslint-plugin/src/__fixtures__/monorepo/packages/bar/package.json b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/bar/package.json new file mode 100644 index 0000000000..8331b20c43 --- /dev/null +++ b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/bar/package.json @@ -0,0 +1,9 @@ +{ + "name": "@internal/bar", + "backstage": { + "role": "frontend-plugin" + }, + "peerDependencies": { + "react": "*" + } +} diff --git a/packages/eslint-plugin/src/__fixtures__/monorepo/packages/foo/package.json b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/foo/package.json new file mode 100644 index 0000000000..66341563cd --- /dev/null +++ b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/foo/package.json @@ -0,0 +1,7 @@ +{ + "name": "@internal/foo", + "dependencies": { + "@internal/bar": "1.0.0", + "lodash": "*" + } +} diff --git a/packages/eslint-plugin/src/no-relative-monorepo-imports.test.ts b/packages/eslint-plugin/src/no-relative-monorepo-imports.test.ts new file mode 100644 index 0000000000..6888d258cc --- /dev/null +++ b/packages/eslint-plugin/src/no-relative-monorepo-imports.test.ts @@ -0,0 +1,68 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { RuleTester } from 'eslint'; +import path from 'path'; +import rule from '../rules/no-relative-monorepo-imports'; + +const RULE = 'no-relative-monorepo-imports'; +const FIXTURE = path.resolve(__dirname, '__fixtures__/monorepo'); + +const ERR_OUTSIDE = (path: string) => ({ + message: `Import of ${path} is outside of any known monorepo package`, +}); +const ERR_FORBIDDEN = (newImp: string) => ({ + message: `Relative imports of monorepo packages are forbidden, use '${newImp}' instead`, +}); + +process.chdir(FIXTURE); + +const ruleTester = new RuleTester({ + parserOptions: { + sourceType: 'module', + ecmaVersion: 2021, + }, +}); + +ruleTester.run(RULE, rule, { + valid: [ + { + code: `import { version } from '@internal/foo'`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + }, + { + code: `import { version } from '@internal/foo/src'`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + }, + ], + invalid: [ + { + code: `import { version } from '../../foo'`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + errors: [ERR_FORBIDDEN('@internal/foo')], + }, + { + code: `import { version } from '../../foo/src'`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + errors: [ERR_FORBIDDEN('@internal/foo/src')], + }, + { + code: `import { version } from '../../../package.json'`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + errors: [ERR_OUTSIDE(path.join(FIXTURE, 'package.json'))], + }, + ], +}); diff --git a/yarn.lock b/yarn.lock index e0e1da349c..2d250e9913 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3721,7 +3721,6 @@ __metadata: eslint-plugin-import: ^2.25.4 eslint-plugin-jest: ^27.0.0 eslint-plugin-jsx-a11y: ^6.5.1 - eslint-plugin-monorepo: ^0.3.2 eslint-plugin-react: ^7.28.0 eslint-plugin-react-hooks: ^4.3.0 eslint-webpack-plugin: ^3.1.1 @@ -4101,6 +4100,7 @@ __metadata: dependencies: "@backstage/cli": "workspace:^" "@manypkg/get-packages": ^1.1.3 + eslint: ^8.33.0 minimatch: ^5.1.2 languageName: unknown linkType: soft @@ -16670,15 +16670,6 @@ __metadata: languageName: node linkType: hard -"array-union@npm:^1.0.1": - version: 1.0.2 - resolution: "array-union@npm:1.0.2" - dependencies: - array-uniq: ^1.0.1 - checksum: 82cec6421b6e6766556c484835a6d476a873f1b71cace5ab2b4f1b15b1e3162dc4da0d16f7a2b04d4aec18146c6638fe8f661340b31ba8e469fd811a1b45dc8d - languageName: node - linkType: hard - "array-union@npm:^2.1.0": version: 2.1.0 resolution: "array-union@npm:2.1.0" @@ -16686,13 +16677,6 @@ __metadata: languageName: node linkType: hard -"array-uniq@npm:^1.0.1": - version: 1.0.3 - resolution: "array-uniq@npm:1.0.3" - checksum: 1625f06b093d8bf279b81adfec6e72951c0857d65b5e3f65f053fffe9f9dd61c2fc52cff57e38a4700817e7e3f01a4faa433d505ea9e33cdae4514c334e0bf9e - languageName: node - linkType: hard - "array.prototype.flat@npm:^1.2.3, array.prototype.flat@npm:^1.3.1": version: 1.3.1 resolution: "array.prototype.flat@npm:1.3.1" @@ -20534,15 +20518,6 @@ __metadata: languageName: node linkType: hard -"dir-glob@npm:^2.0.0": - version: 2.2.2 - resolution: "dir-glob@npm:2.2.2" - dependencies: - path-type: ^3.0.0 - checksum: 3aa48714a9f7845ffc30ab03a5c674fe760477cc55e67b0847333371549227d93953e6627ec160f75140c5bea5c5f88d13c01de79bd1997a588efbcf06980842 - languageName: node - linkType: hard - "dir-glob@npm:^3.0.1": version: 3.0.1 resolution: "dir-glob@npm:3.0.1" @@ -21606,7 +21581,7 @@ __metadata: languageName: node linkType: hard -"eslint-module-utils@npm:^2.1.1, eslint-module-utils@npm:^2.7.4": +"eslint-module-utils@npm:^2.7.4": version: 2.7.4 resolution: "eslint-module-utils@npm:2.7.4" dependencies: @@ -21711,21 +21686,6 @@ __metadata: languageName: node linkType: hard -"eslint-plugin-monorepo@npm:^0.3.2": - version: 0.3.2 - resolution: "eslint-plugin-monorepo@npm:0.3.2" - dependencies: - eslint-module-utils: ^2.1.1 - get-monorepo-packages: ^1.1.0 - globby: ^7.1.1 - load-json-file: ^4.0.0 - minimatch: ^3.0.4 - parse-package-name: ^0.1.0 - path-is-inside: ^1.0.2 - checksum: b6f17efbc9e66aefbf7aacf9aa130bc8e729a6311c412d1b0026ce7d93361c10b0e6e4b10cf434f24ca9a8b2e891bc8e5b82e66dc9ccc5e782f9c18a989eca28 - languageName: node - linkType: hard - "eslint-plugin-notice@npm:^0.9.10": version: 0.9.10 resolution: "eslint-plugin-notice@npm:0.9.10" @@ -21845,7 +21805,7 @@ __metadata: languageName: node linkType: hard -"eslint@npm:^8.6.0": +"eslint@npm:^8.33.0, eslint@npm:^8.6.0": version: 8.33.0 resolution: "eslint@npm:8.33.0" dependencies: @@ -23438,16 +23398,6 @@ __metadata: languageName: node linkType: hard -"get-monorepo-packages@npm:^1.1.0": - version: 1.2.0 - resolution: "get-monorepo-packages@npm:1.2.0" - dependencies: - globby: ^7.1.1 - load-json-file: ^4.0.0 - checksum: f9321c11b8e11f02138758db6589d8ab8b7e1b05e78cac92493b635faea10aa100c20fe40f2cf110c82ec100c118c6c131dff4c65d32a721c617dc2928f7b277 - languageName: node - linkType: hard - "get-package-type@npm:^0.1.0": version: 0.1.0 resolution: "get-package-type@npm:0.1.0" @@ -23598,7 +23548,7 @@ __metadata: languageName: node linkType: hard -"glob@npm:^7.0.0, glob@npm:^7.1.1, glob@npm:^7.1.2, glob@npm:^7.1.3, glob@npm:^7.1.4, glob@npm:^7.1.6, glob@npm:^7.1.7, glob@npm:^7.2.0": +"glob@npm:^7.0.0, glob@npm:^7.1.1, glob@npm:^7.1.3, glob@npm:^7.1.4, glob@npm:^7.1.6, glob@npm:^7.1.7, glob@npm:^7.2.0": version: 7.2.3 resolution: "glob@npm:7.2.3" dependencies: @@ -23707,20 +23657,6 @@ __metadata: languageName: node linkType: hard -"globby@npm:^7.1.1": - version: 7.1.1 - resolution: "globby@npm:7.1.1" - dependencies: - array-union: ^1.0.1 - dir-glob: ^2.0.0 - glob: ^7.1.2 - ignore: ^3.3.5 - pify: ^3.0.0 - slash: ^1.0.0 - checksum: f0eba08a08ae7c98149a4411661c0bf08c4717d81e6f355cf624fb01880b249737eb8e951bf86124cb3af8ea1c793c0a9d363ed5cdec99bb2c6b68f8a323025f - languageName: node - linkType: hard - "good-listener@npm:^1.2.2": version: 1.2.2 resolution: "good-listener@npm:1.2.2" @@ -24710,13 +24646,6 @@ __metadata: languageName: node linkType: hard -"ignore@npm:^3.3.5": - version: 3.3.10 - resolution: "ignore@npm:3.3.10" - checksum: 23e8cc776e367b56615ab21b78decf973a35dfca5522b39d9b47643d8168473b0d1f18dd1321a1bab466a12ea11a2411903f3b21644f4d5461ee0711ec8678bd - languageName: node - linkType: hard - "ignore@npm:^5.1.4, ignore@npm:^5.2.0": version: 5.2.0 resolution: "ignore@npm:5.2.0" @@ -26828,13 +26757,6 @@ __metadata: languageName: node linkType: hard -"json-parse-better-errors@npm:^1.0.1": - version: 1.0.2 - resolution: "json-parse-better-errors@npm:1.0.2" - checksum: ff2b5ba2a70e88fd97a3cb28c1840144c5ce8fae9cbeeddba15afa333a5c407cf0e42300cd0a2885dbb055227fe68d405070faad941beeffbfde9cf3b2c78c5d - languageName: node - linkType: hard - "json-parse-even-better-errors@npm:^2.3.0, json-parse-even-better-errors@npm:^2.3.1": version: 2.3.1 resolution: "json-parse-even-better-errors@npm:2.3.1" @@ -27725,18 +27647,6 @@ __metadata: languageName: node linkType: hard -"load-json-file@npm:^4.0.0": - version: 4.0.0 - resolution: "load-json-file@npm:4.0.0" - dependencies: - graceful-fs: ^4.1.2 - parse-json: ^4.0.0 - pify: ^3.0.0 - strip-bom: ^3.0.0 - checksum: 8f5d6d93ba64a9620445ee9bde4d98b1eac32cf6c8c2d20d44abfa41a6945e7969456ab5f1ca2fb06ee32e206c9769a20eec7002fe290de462e8c884b6b8b356 - languageName: node - linkType: hard - "load-yaml-file@npm:^0.2.0": version: 0.2.0 resolution: "load-yaml-file@npm:0.2.0" @@ -30849,16 +30759,6 @@ __metadata: languageName: node linkType: hard -"parse-json@npm:^4.0.0": - version: 4.0.0 - resolution: "parse-json@npm:4.0.0" - dependencies: - error-ex: ^1.3.1 - json-parse-better-errors: ^1.0.1 - checksum: 0fe227d410a61090c247e34fa210552b834613c006c2c64d9a05cfe9e89cf8b4246d1246b1a99524b53b313e9ac024438d0680f67e33eaed7e6f38db64cfe7b5 - languageName: node - linkType: hard - "parse-json@npm:^5.0.0, parse-json@npm:^5.2.0": version: 5.2.0 resolution: "parse-json@npm:5.2.0" @@ -30871,13 +30771,6 @@ __metadata: languageName: node linkType: hard -"parse-package-name@npm:^0.1.0": - version: 0.1.0 - resolution: "parse-package-name@npm:0.1.0" - checksum: 7d69b1deeaf82d5a59536c5a8772c8db003b1415dd2465073343dfab5caf90917ecfffdd71d1c004f5cf963c3881ccb8899d0ff820eac705a272e817e246de14 - languageName: node - linkType: hard - "parse-path@npm:^7.0.0": version: 7.0.0 resolution: "parse-path@npm:7.0.0" @@ -31150,7 +31043,7 @@ __metadata: languageName: node linkType: hard -"path-is-inside@npm:1.0.2, path-is-inside@npm:^1.0.2": +"path-is-inside@npm:1.0.2": version: 1.0.2 resolution: "path-is-inside@npm:1.0.2" checksum: 0b5b6c92d3018b82afb1f74fe6de6338c4c654de4a96123cb343f2b747d5606590ac0c890f956ed38220a4ab59baddfd7b713d78a62d240b20b14ab801fa02cb @@ -31249,15 +31142,6 @@ __metadata: languageName: node linkType: hard -"path-type@npm:^3.0.0": - version: 3.0.0 - resolution: "path-type@npm:3.0.0" - dependencies: - pify: ^3.0.0 - checksum: 735b35e256bad181f38fa021033b1c33cfbe62ead42bb2222b56c210e42938eecb272ae1949f3b6db4ac39597a61b44edd8384623ec4d79bfdc9a9c0f12537a6 - languageName: node - linkType: hard - "path-type@npm:^4.0.0": version: 4.0.0 resolution: "path-type@npm:4.0.0" @@ -31432,13 +31316,6 @@ __metadata: languageName: node linkType: hard -"pify@npm:^3.0.0": - version: 3.0.0 - resolution: "pify@npm:3.0.0" - checksum: 6cdcbc3567d5c412450c53261a3f10991665d660961e06605decf4544a61a97a54fefe70a68d5c37080ff9d6f4cf51444c90198d1ba9f9309a6c0d6e9f5c4fde - languageName: node - linkType: hard - "pify@npm:^4.0.1": version: 4.0.1 resolution: "pify@npm:4.0.1" @@ -34985,13 +34862,6 @@ __metadata: languageName: node linkType: hard -"slash@npm:^1.0.0": - version: 1.0.0 - resolution: "slash@npm:1.0.0" - checksum: 4b6e21b1fba6184a7e2efb1dd173f692d8a845584c1bbf9dc818ff86f5a52fc91b413008223d17cc684604ee8bb9263a420b1182027ad9762e35388434918860 - languageName: node - linkType: hard - "slash@npm:^2.0.0": version: 2.0.0 resolution: "slash@npm:2.0.0" From 6801eb57805c06c398bd82da72f1429acc634a07 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 4 Feb 2023 10:52:55 +0100 Subject: [PATCH 19/25] eslint-plugin: refactor to fix issues uncovered by more strict parsing Signed-off-by: Patrik Oldsberg --- packages/eslint-plugin/lib/getPackages.js | 12 ++++++------ packages/eslint-plugin/lib/visitImports.js | 12 ++++++++++-- .../eslint-plugin/rules/no-undeclared-imports.js | 13 +++++++++---- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/lib/getPackages.js b/packages/eslint-plugin/lib/getPackages.js index 6c8c1a66b2..f5f2c63cc1 100644 --- a/packages/eslint-plugin/lib/getPackages.js +++ b/packages/eslint-plugin/lib/getPackages.js @@ -36,8 +36,10 @@ const manypkg = require('@manypkg/get-packages'); // Loads all packages in the monorepo once, and caches the result module.exports = (function () { + /** @type {PackageMap | undefined} */ let result = undefined; - let lastLoadAt = undefined; + /** @type {number} */ + let lastLoadAt = 0; /** @returns {PackageMap | undefined} */ return function getPackages(/** @type {string} */ dir) { @@ -58,11 +60,9 @@ module.exports = (function () { list: packages.packages, root: packages.root, byPath(filePath) { - return packages.packages.find(pkg => { - if (!path.relative(pkg.dir, filePath).startsWith('..')) { - return pkg; - } - }); + return packages.packages.find( + pkg => !path.relative(pkg.dir, filePath).startsWith('..'), + ); }, }; lastLoadAt = Date.now(); diff --git a/packages/eslint-plugin/lib/visitImports.js b/packages/eslint-plugin/lib/visitImports.js index ed16c26c7f..43401bf5f9 100644 --- a/packages/eslint-plugin/lib/visitImports.js +++ b/packages/eslint-plugin/lib/visitImports.js @@ -56,12 +56,17 @@ const getPackages = require('./getPackages'); /** * @callback ImportVisitor - * @param {import('eslint').Rule.Node} node + * @param {ConsideredNode} node * @param {LocalImport | InternalImport | ExternalImport | BuiltinImport} import */ /** - * @param {import('estree').ImportDeclaration | import('estree').ExportAllDeclaration | import('estree').ExportNamedDeclaration | import('estree').ImportExpression | import('estree').SimpleCallExpression} node + * @typedef ConsideredNode + * @type {import('estree').ImportDeclaration | import('estree').ExportAllDeclaration | import('estree').ExportNamedDeclaration | import('estree').ImportExpression | import('estree').SimpleCallExpression} + */ + +/** + * @param {ConsideredNode} node * @returns {undefined | {path: string, kind: 'type' | 'value'}} */ function getImportInfo(node) { @@ -103,6 +108,9 @@ module.exports = function visitImports(context, visitor) { return; } + /** + * @param {ConsideredNode} node + */ function visit(node) { const info = getImportInfo(node); if (!info) { diff --git a/packages/eslint-plugin/rules/no-undeclared-imports.js b/packages/eslint-plugin/rules/no-undeclared-imports.js index 4f60b712d9..85d7266eb8 100644 --- a/packages/eslint-plugin/rules/no-undeclared-imports.js +++ b/packages/eslint-plugin/rules/no-undeclared-imports.js @@ -37,7 +37,7 @@ const devModulePatterns = [ ]; function getExpectedDepType( - localPkg, + /** @type {any} */ localPkg, /** @type {string} */ impPath, /** @type {string} */ modulePath, ) { @@ -58,6 +58,7 @@ function getExpectedDepType( case 'react-router-dom': return 'peer'; } + break; case 'cli': case 'frontend': case 'backend': @@ -100,10 +101,14 @@ function findConflict(pkg, name, expectedType) { return { oldDepsField, depsField }; } + return undefined; } -function getAddFlagForConflict(conflict) { - switch (conflict?.depsField) { +/** + * @param {string} depsField + */ +function getAddFlagForDepsField(depsField) { + switch (depsField) { case depFields.dep: return ''; case depFields.dev: @@ -189,7 +194,7 @@ module.exports = { data: { ...conflict, packagePath, - addFlag: getAddFlagForConflict(conflict), + addFlag: getAddFlagForDepsField(conflict.depsField), packageName: imp.packageName, packageJsonPath: packageJsonPath, }, From e6d210947ece9b5fcaa90698f2b577936db3445c Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 4 Feb 2023 11:15:52 +0100 Subject: [PATCH 20/25] eslint-plugin: add tests for no-undeclared-imports + fixes Signed-off-by: Patrik Oldsberg --- packages/eslint-plugin/.eslintrc.js | 3 + .../rules/no-undeclared-imports.js | 2 +- .../monorepo/packages/bar/package.json | 7 +- .../monorepo/packages/foo/package.json | 7 +- .../src/no-undeclared-imports.test.ts | 194 ++++++++++++++++++ 5 files changed, 209 insertions(+), 4 deletions(-) create mode 100644 packages/eslint-plugin/src/no-undeclared-imports.test.ts diff --git a/packages/eslint-plugin/.eslintrc.js b/packages/eslint-plugin/.eslintrc.js index 01d1b181d7..6b9de4544e 100644 --- a/packages/eslint-plugin/.eslintrc.js +++ b/packages/eslint-plugin/.eslintrc.js @@ -21,8 +21,11 @@ module.exports = { node: true, es2021: true, }, + parser: '@typescript-eslint/parser', parserOptions: { + ecmaVersion: 2018, sourceType: 'module', + lib: require('@backstage/cli/config/tsconfig.json').compilerOptions.lib, }, rules: { '@backstage/no-undeclared-imports': ['error'], diff --git a/packages/eslint-plugin/rules/no-undeclared-imports.js b/packages/eslint-plugin/rules/no-undeclared-imports.js index 85d7266eb8..42624beb5c 100644 --- a/packages/eslint-plugin/rules/no-undeclared-imports.js +++ b/packages/eslint-plugin/rules/no-undeclared-imports.js @@ -161,7 +161,7 @@ module.exports = { const modulePath = path.relative(localPkg.dir, filePath); const expectedType = getExpectedDepType( - localPkg, + localPkg.packageJson, imp.packageName, modulePath, ); diff --git a/packages/eslint-plugin/src/__fixtures__/monorepo/packages/bar/package.json b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/bar/package.json index 8331b20c43..c876e00e32 100644 --- a/packages/eslint-plugin/src/__fixtures__/monorepo/packages/bar/package.json +++ b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/bar/package.json @@ -3,7 +3,10 @@ "backstage": { "role": "frontend-plugin" }, - "peerDependencies": { - "react": "*" + "dependencies": { + "react-router": "*" + }, + "devDependencies": { + "react-router-dom": "*" } } diff --git a/packages/eslint-plugin/src/__fixtures__/monorepo/packages/foo/package.json b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/foo/package.json index 66341563cd..8a14cad4d3 100644 --- a/packages/eslint-plugin/src/__fixtures__/monorepo/packages/foo/package.json +++ b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/foo/package.json @@ -1,7 +1,12 @@ { "name": "@internal/foo", "dependencies": { - "@internal/bar": "1.0.0", + "@internal/bar": "1.0.0" + }, + "devDependencies": { "lodash": "*" + }, + "peerDependencies": { + "react": "*" } } diff --git a/packages/eslint-plugin/src/no-undeclared-imports.test.ts b/packages/eslint-plugin/src/no-undeclared-imports.test.ts new file mode 100644 index 0000000000..c156558e8d --- /dev/null +++ b/packages/eslint-plugin/src/no-undeclared-imports.test.ts @@ -0,0 +1,194 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { RuleTester } from 'eslint'; +import path from 'path'; +import rule from '../rules/no-undeclared-imports'; + +const RULE = 'no-undeclared-imports'; +const FIXTURE = path.resolve(__dirname, '__fixtures__/monorepo'); + +const ERR_UNDECLARED = ( + name: string, + field: string, + path: string, + flag?: string, +) => ({ + message: `${name} must be declared in ${field} of ${path}/package.json, run 'yarn --cwd ${path} add${ + flag ? ` ${flag}` : '' + } ${name}' from the project root.`, +}); +const ERR_SWITCHED = ( + name: string, + old: string, + field: string, + path: string, +) => ({ + message: `${name} is declared in ${old}, but should be moved to ${field} in ${path}/package.json.`, +}); + +process.chdir(FIXTURE); + +const ruleTester = new RuleTester({ + parserOptions: { + sourceType: 'module', + ecmaVersion: 2021, + }, +}); + +ruleTester.run(RULE, rule, { + valid: [ + { + code: `import '@internal/foo'`, + filename: path.join(FIXTURE, 'packages/foo/src/index.ts'), + }, + { + code: `import '@internal/bar'`, + filename: path.join(FIXTURE, 'packages/foo/src/index.ts'), + }, + { + code: `import 'react'`, + filename: path.join(FIXTURE, 'packages/foo/src/index.ts'), + }, + { + code: `import '@internal/foo'`, + filename: path.join(FIXTURE, 'packages/foo/src/index.test.ts'), + }, + { + code: `import '@internal/bar'`, + filename: path.join(FIXTURE, 'packages/foo/src/index.test.ts'), + }, + { + code: `import 'lodash'`, + filename: path.join(FIXTURE, 'packages/foo/src/index.test.ts'), + }, + { + code: `import 'react'`, + filename: path.join(FIXTURE, 'packages/foo/src/index.test.ts'), + }, + { + // We're only able to validate literals + code: `require('lod' + 'ash')`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + }, + ], + invalid: [ + { + code: `import 'lodash'`, + filename: path.join(FIXTURE, 'packages/foo/src/index.ts'), + errors: [ + ERR_SWITCHED( + 'lodash', + 'devDependencies', + 'dependencies', + 'packages/foo', + ), + ], + }, + { + code: `import 'react-router'`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + errors: [ + ERR_SWITCHED( + 'react-router', + 'dependencies', + 'peerDependencies', + 'packages/bar', + ), + ], + }, + { + code: `import 'react-router-dom'`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + errors: [ + ERR_SWITCHED( + 'react-router-dom', + 'devDependencies', + 'peerDependencies', + 'packages/bar', + ), + ], + }, + { + code: `import 'lodash'`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + errors: [ERR_UNDECLARED('lodash', 'dependencies', 'packages/bar')], + }, + { + code: `import { debounce } from 'lodash'`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + errors: [ERR_UNDECLARED('lodash', 'dependencies', 'packages/bar')], + }, + { + code: `import * as _ from 'lodash'`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + errors: [ERR_UNDECLARED('lodash', 'dependencies', 'packages/bar')], + }, + { + code: `import _ from 'lodash'`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + errors: [ERR_UNDECLARED('lodash', 'dependencies', 'packages/bar')], + }, + { + code: `import('lodash')`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + errors: [ERR_UNDECLARED('lodash', 'dependencies', 'packages/bar')], + }, + { + code: `require('lodash')`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + errors: [ERR_UNDECLARED('lodash', 'dependencies', 'packages/bar')], + }, + { + code: `import 'lodash'`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + errors: [ERR_UNDECLARED('lodash', 'dependencies', 'packages/bar')], + }, + { + code: `import 'lodash'`, + filename: path.join(FIXTURE, 'packages/bar/src/index.test.ts'), + errors: [ + ERR_UNDECLARED('lodash', 'devDependencies', 'packages/bar', '--dev'), + ], + }, + { + code: `import 'react'`, + filename: path.join(FIXTURE, 'packages/bar/src/index.ts'), + errors: [ + ERR_UNDECLARED('react', 'peerDependencies', 'packages/bar', '--peer'), + ], + }, + { + code: `import 'react'`, + filename: path.join(FIXTURE, 'packages/bar/src/index.test.ts'), + errors: [ + ERR_UNDECLARED('react', 'peerDependencies', 'packages/bar', '--peer'), + ], + }, + { + code: `import 'react-dom'`, + filename: path.join(FIXTURE, 'packages/foo/src/index.ts'), + errors: [ERR_UNDECLARED('react-dom', 'dependencies', 'packages/foo')], + }, + { + code: `import 'react-dom'`, + filename: path.join(FIXTURE, 'packages/foo/src/index.test.ts'), + errors: [ + ERR_UNDECLARED('react-dom', 'devDependencies', 'packages/foo', '--dev'), + ], + }, + ], +}); From 2638dd4e7c9aa88ffcf987a33ff86465292094c4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 4 Feb 2023 11:31:00 +0100 Subject: [PATCH 21/25] eslint-plugin: add tests for no-forbidden-package-imports Signed-off-by: Patrik Oldsberg --- .../monorepo/packages/bar/package.json | 5 + .../monorepo/packages/foo/package.json | 6 +- .../src/no-forbidden-package-imports.test.ts | 120 ++++++++++++++++++ 3 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 packages/eslint-plugin/src/no-forbidden-package-imports.test.ts diff --git a/packages/eslint-plugin/src/__fixtures__/monorepo/packages/bar/package.json b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/bar/package.json index c876e00e32..efef27047b 100644 --- a/packages/eslint-plugin/src/__fixtures__/monorepo/packages/bar/package.json +++ b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/bar/package.json @@ -1,5 +1,10 @@ { "name": "@internal/bar", + "exports": { + ".": "./src/index.ts", + "./BarPage": "./src/components/Bar.tsx", + "./package.json": "./package.json" + }, "backstage": { "role": "frontend-plugin" }, diff --git a/packages/eslint-plugin/src/__fixtures__/monorepo/packages/foo/package.json b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/foo/package.json index 8a14cad4d3..563ec35120 100644 --- a/packages/eslint-plugin/src/__fixtures__/monorepo/packages/foo/package.json +++ b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/foo/package.json @@ -8,5 +8,9 @@ }, "peerDependencies": { "react": "*" - } + }, + "files": [ + "dist", + "type-utils" + ] } diff --git a/packages/eslint-plugin/src/no-forbidden-package-imports.test.ts b/packages/eslint-plugin/src/no-forbidden-package-imports.test.ts new file mode 100644 index 0000000000..349b71319f --- /dev/null +++ b/packages/eslint-plugin/src/no-forbidden-package-imports.test.ts @@ -0,0 +1,120 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { RuleTester } from 'eslint'; +import path from 'path'; +import rule from '../rules/no-forbidden-package-imports'; + +const RULE = 'no-forbidden-package-imports'; +const FIXTURE = path.resolve(__dirname, '__fixtures__/monorepo'); + +const ERR = (name: string, path: string) => ({ + message: `${name} does not export ${path}`, +}); + +process.chdir(FIXTURE); + +const ruleTester = new RuleTester({ + parserOptions: { + sourceType: 'module', + ecmaVersion: 2021, + }, +}); + +ruleTester.run(RULE, rule, { + valid: [ + { + code: `import '@internal/foo'`, + filename: path.join(FIXTURE, 'index.ts'), + }, + { + code: `import '@internal/foo/type-utils'`, + filename: path.join(FIXTURE, 'index.ts'), + }, + { + code: `import '@internal/foo/type-utils/anything'`, + filename: path.join(FIXTURE, 'index.ts'), + }, + { + code: `import '@internal/foo/package.json'`, + filename: path.join(FIXTURE, 'index.ts'), + }, + { + code: `import '@internal/bar'`, + filename: path.join(FIXTURE, 'index.ts'), + }, + { + code: `import '@internal/bar/package.json'`, + filename: path.join(FIXTURE, 'index.ts'), + }, + { + code: `import '@internal/bar/BarPage'`, + filename: path.join(FIXTURE, 'index.ts'), + }, + ], + invalid: [ + { + code: `import '@internal/foo/FooPage'`, + filename: path.join(FIXTURE, 'index.ts'), + errors: [ERR('@internal/foo', 'FooPage')], + }, + { + code: `import '@internal/foo/dist'`, + filename: path.join(FIXTURE, 'index.ts'), + errors: [ERR('@internal/foo', 'dist')], + }, + { + code: `import '@internal/foo/dist/FooPage'`, + filename: path.join(FIXTURE, 'index.ts'), + errors: [ERR('@internal/foo', 'dist/FooPage')], + }, + { + code: `import '@internal/foo/src/FooPage'`, + filename: path.join(FIXTURE, 'index.ts'), + errors: [ERR('@internal/foo', 'src/FooPage')], + }, + { + code: `import '@internal/foo/src'`, + filename: path.join(FIXTURE, 'index.ts'), + errors: [ERR('@internal/foo', 'src')], + }, + { + code: `import '@internal/bar/OtherBarPage'`, + filename: path.join(FIXTURE, 'index.ts'), + errors: [ERR('@internal/bar', 'OtherBarPage')], + }, + { + code: `import '@internal/bar/dist'`, + filename: path.join(FIXTURE, 'index.ts'), + errors: [ERR('@internal/bar', 'dist')], + }, + { + code: `import '@internal/bar/dist/BarPage'`, + filename: path.join(FIXTURE, 'index.ts'), + errors: [ERR('@internal/bar', 'dist/BarPage')], + }, + { + code: `import '@internal/bar/src/BarPage'`, + filename: path.join(FIXTURE, 'index.ts'), + errors: [ERR('@internal/bar', 'src/BarPage')], + }, + { + code: `import '@internal/bar/src'`, + filename: path.join(FIXTURE, 'index.ts'), + errors: [ERR('@internal/bar', 'src')], + }, + ], +}); From 02f1316e57d709c7e8e458fa93828ef9ed1e96f2 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 4 Feb 2023 13:09:13 +0100 Subject: [PATCH 22/25] changeset: added changesets for eslint plugin fixes that affect the published package Signed-off-by: Patrik Oldsberg --- .changeset/grumpy-fireants-drop.md | 5 +++++ .changeset/wicked-beers-walk.md | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 .changeset/grumpy-fireants-drop.md create mode 100644 .changeset/wicked-beers-walk.md diff --git a/.changeset/grumpy-fireants-drop.md b/.changeset/grumpy-fireants-drop.md new file mode 100644 index 0000000000..92b08663f4 --- /dev/null +++ b/.changeset/grumpy-fireants-drop.md @@ -0,0 +1,5 @@ +--- +'@backstage/codemods': patch +--- + +Moved `commander` to being a regular dependency. diff --git a/.changeset/wicked-beers-walk.md b/.changeset/wicked-beers-walk.md new file mode 100644 index 0000000000..160a171eb1 --- /dev/null +++ b/.changeset/wicked-beers-walk.md @@ -0,0 +1,5 @@ +--- +'@backstage/create-app': patch +--- + +Updated `packages/app/cypress/.eslintrc.json` to remove the unnecessary `import/no-extraneous-dependencies` rule. From ff63acf30a4bc94377b5c4f220deebe07af4d915 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 4 Feb 2023 13:12:16 +0100 Subject: [PATCH 23/25] repo-tools: make api extraction ignore packages without a role Signed-off-by: Patrik Oldsberg --- .changeset/rare-berries-give.md | 5 +++++ .../repo-tools/src/commands/api-reports/api-extractor.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changeset/rare-berries-give.md diff --git a/.changeset/rare-berries-give.md b/.changeset/rare-berries-give.md new file mode 100644 index 0000000000..39e360bf48 --- /dev/null +++ b/.changeset/rare-berries-give.md @@ -0,0 +1,5 @@ +--- +'@backstage/repo-tools': patch +--- + +Packages without a declared `backstage.role` are now ignored. diff --git a/packages/repo-tools/src/commands/api-reports/api-extractor.ts b/packages/repo-tools/src/commands/api-reports/api-extractor.ts index b037f40be6..39df70c21a 100644 --- a/packages/repo-tools/src/commands/api-reports/api-extractor.ts +++ b/packages/repo-tools/src/commands/api-reports/api-extractor.ts @@ -1107,7 +1107,7 @@ export async function categorizePackageDirs(packageDirs: any[]) { }); const role = pkgJson?.backstage?.role; if (!role) { - throw new Error(`No backstage.role in ${dir}/package.json`); + return; // Ignore packages without roles } if (role === 'cli') { cliPackageDirs.push(dir); From dec4896baa9182120671195fe1fc81008deacbac Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 4 Feb 2023 13:13:11 +0100 Subject: [PATCH 24/25] cli: add missing @backstage/eslint-plugin dependency Signed-off-by: Patrik Oldsberg --- packages/cli/package.json | 1 + yarn.lock | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index 359f16adfa..2f5292e3a0 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -34,6 +34,7 @@ "@backstage/config": "workspace:^", "@backstage/config-loader": "workspace:^", "@backstage/errors": "workspace:^", + "@backstage/eslint-plugin": "workspace:^", "@backstage/release-manifests": "workspace:^", "@backstage/types": "workspace:^", "@manypkg/get-packages": "^1.1.3", diff --git a/yarn.lock b/yarn.lock index 2d250e9913..069b1125a2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3660,6 +3660,7 @@ __metadata: "@backstage/core-plugin-api": "workspace:^" "@backstage/dev-utils": "workspace:^" "@backstage/errors": "workspace:^" + "@backstage/eslint-plugin": "workspace:^" "@backstage/release-manifests": "workspace:^" "@backstage/test-utils": "workspace:^" "@backstage/theme": "workspace:^" @@ -4094,7 +4095,7 @@ __metadata: languageName: unknown linkType: soft -"@backstage/eslint-plugin@workspace:packages/eslint-plugin": +"@backstage/eslint-plugin@workspace:^, @backstage/eslint-plugin@workspace:packages/eslint-plugin": version: 0.0.0-use.local resolution: "@backstage/eslint-plugin@workspace:packages/eslint-plugin" dependencies: From baf6e4c96a90b54a52e8acc28e2dcaf706d63bd4 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sat, 4 Feb 2023 13:57:18 +0100 Subject: [PATCH 25/25] backend-test-utils: removed unnecessary @backstage/cli dependency Signed-off-by: Patrik Oldsberg --- .changeset/chilled-sheep-collect.md | 5 +++++ packages/backend-test-utils/package.json | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 .changeset/chilled-sheep-collect.md diff --git a/.changeset/chilled-sheep-collect.md b/.changeset/chilled-sheep-collect.md new file mode 100644 index 0000000000..b944685e16 --- /dev/null +++ b/.changeset/chilled-sheep-collect.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-test-utils': patch +--- + +Removed unnecessary `@backstage/cli` dependency. diff --git a/packages/backend-test-utils/package.json b/packages/backend-test-utils/package.json index a01dbaf0fa..d1f0a86d43 100644 --- a/packages/backend-test-utils/package.json +++ b/packages/backend-test-utils/package.json @@ -37,7 +37,6 @@ "@backstage/backend-app-api": "workspace:^", "@backstage/backend-common": "workspace:^", "@backstage/backend-plugin-api": "workspace:^", - "@backstage/cli": "workspace:^", "@backstage/config": "workspace:^", "@backstage/plugin-auth-node": "workspace:^", "@backstage/types": "workspace:^",