Merge pull request #31373 from drodil/nfs_imports

feat(eslint): relax frontend imports for nfs

.changeset/hungry-needles-clean.md

@@ -0,0 +1,8 @@
+---
+'@backstage/eslint-plugin': patch
+---
+
+Allow frontend plugin to import from another frontend plugin with same plugin id.
+
+This prevents the ESLint rule from incorrectly flagging these imports in the new frontend system
+where plugin override requires cross-plugin imports.

packages/eslint-plugin/docs/rules/no-mixed-plugin-imports.md

@@ -1,6 +1,25 @@
 # @backstage/no-mixed-plugin-imports
 
-Disallow mixed imports between backstage plugins.
+This rule ensures that imports between Backstage plugins are consistent with their intended usage.
+
+The rule checks the `backstage.role` field in the `package.json` of both the importing and target packages to determine if they are compatible.
+
+Plugin roles include:
+
+- `frontend-plugin` (or `frontend` for short)
+- `backend-plugin` (or `backend` for short)
+- `web-library` (or `react` for short)
+- `node-library` (or `node` for short)
+- `common-library` (or `common` for short)
+
+Prohibited imports include:
+
+- A `frontend` plugin importing directly from another `frontend`, `backend`, or `node` package. Instead, it should import from the corresponding `react` or `common` package.
+  - With an exception with the new frontend system where frontend plugins with the same plugin id are allowed to import from each other.
+- A `backend` plugin importing directly from another `backend`, `frontend`, or `react` package. Instead, it should import from the corresponding `node` or `common` package.
+- A `react` package importing from `frontend`, `backend`, or `node` packages.
+- A `node` package importing from `frontend`, `backend`, or `react` packages
+- A `common` package importing directly from any other plugin package.
 
 ## Usage
 

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<string, string>, files?: Array<string>, backstage?: { inline?: boolean, role?: string } }}} packageJson
+ * @type {import('@manypkg/get-packages').Package & { packageJson: { exports?: Record<string, string>, files?: Array<string>, backstage?: { inline?: boolean, role?: string, pluginId?: string } }}} packageJson
  */
 
 /**

packages/eslint-plugin/rules/no-mixed-plugin-imports.js

@@ -64,6 +64,7 @@ module.exports = {
     messages: {
       forbidden:
         '{{sourcePackage}} ({{sourceRole}}) uses forbidden import from {{targetPackage}} ({{targetRole}}).',
+      useSamePluginId: `Import of {{targetPackage}} ({{targetRole}}) from {{sourceRole}} is forbidden unless you are overriding the plugin, in which case the \`backstage.pluginId\` in {{sourcePackage}}/package.json must be the same as in {{targetPackage}}`,
       useReactPlugin:
         'Use web library {{targetPackage}}-react or common library instead.',
       useNodePlugin:
@@ -146,11 +147,24 @@ module.exports = {
       }
 
       const sourceRole = pkg.packageJson.backstage?.role;
+      const sourcePluginId = pkg.packageJson.backstage?.pluginId;
       const targetRole = targetPackage.packageJson.backstage?.role;
+      const targetPluginId = targetPackage.packageJson.backstage?.pluginId;
       if (!sourceRole || !targetRole) {
         return;
       }
 
+      // Allow frontend plugins to import from other frontend plugins with the same pluginId for NFS
+      if (
+        sourceRole === 'frontend-plugin' &&
+        targetRole === 'frontend-plugin' &&
+        sourcePluginId &&
+        targetPluginId &&
+        sourcePluginId === targetPluginId
+      ) {
+        return;
+      }
+
       if (
         roleRules.some(
           rule =>
@@ -165,6 +179,19 @@ module.exports = {
           (sourceRole === 'frontend-plugin' || sourceRole === 'web-library') &&
           targetRole === 'frontend-plugin'
         ) {
+          suggest.push({
+            messageId: 'useSamePluginId',
+            data: {
+              targetPackage: targetName,
+              targetRole: targetRole,
+              sourcePackage: sourceName,
+              sourceRole: sourceRole,
+            },
+            /** @param {import('eslint').Rule.RuleFixer} _fixer */
+            fix(_fixer) {
+              // Not a fixable case, just give a suggestion to change plugin id
+            },
+          });
           suggest.push({
             messageId: 'useReactPlugin',
             data: {

packages/eslint-plugin/src/__fixtures__/monorepo/packages/bar-override/package.json

@@ -0,0 +1,19 @@
+{
+  "name": "@internal/bar-override",
+  "backstage": {
+    "role": "frontend-plugin",
+    "pluginId": "bar"
+  },
+  "exports": {
+    ".": "./src/index.ts",
+    "./BarPage": "./src/components/Bar.tsx",
+    "./package.json": "./package.json"
+  },
+  "dependencies": {
+    "inline-dep": "*",
+    "react-router": "*"
+  },
+  "devDependencies": {
+    "react-router-dom": "*"
+  }
+}

packages/eslint-plugin/src/__fixtures__/monorepo/packages/bar/package.json

@@ -1,7 +1,8 @@
 {
   "name": "@internal/bar",
   "backstage": {
-    "role": "frontend-plugin"
+    "role": "frontend-plugin",
+    "pluginId": "bar"
   },
   "exports": {
     ".": "./src/index.ts",

packages/eslint-plugin/src/__fixtures__/monorepo/packages/foo/package.json

@@ -1,7 +1,8 @@
 {
   "name": "@internal/foo",
   "backstage": {
-    "role": "frontend-plugin"
+    "role": "frontend-plugin",
+    "pluginId": "foo"
   },
   "files": [
     "dist",