cli: switch rollup config to consider all package modules external

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

.changeset/tender-berries-lie.md

@@ -0,0 +1,5 @@
+---
+'@backstage/cli': patch
+---
+
+Changed the logic for how modules are marked as external in the Rollup build of packages. Rather than only marking dependencies and build-in Node.js modules as external, all non-relative imports are now considered external.

docs/local-dev/cli-build-system.md

@@ -181,12 +181,10 @@ files like stylesheets or images. For more details on what syntax and file
 formats are supported by the build process, see the [loaders section](#loaders).
 
 When building CommonJS or ESM output, the build commands will always use
-`src/index.ts` as the entrypoint. All dependencies of the package will be marked
-as external, meaning that in general it is only the contents of the `src` folder
-that ends up being compiled and output to `dist`. All import statements of
-external dependencies, even within the same monorepo, will stay intact. The
-externalized dependencies are based on dependency information in `package.json`,
-which means it's important to keep it up to date.
+`src/index.ts` as the entrypoint. All non-relative modules imports are considered
+external, meaning the Rollup build will only compile the source code of the package
+itself. All import statements of external dependencies, even within the same
+monorepo, will stay intact.
 
 The build of the type definitions works quite differently. The entrypoint of the
 type definition build is the relative location of the package within the

packages/cli/package.json

@@ -103,7 +103,6 @@
     "rollup": "^2.60.2",
     "rollup-plugin-dts": "^4.0.1",
     "rollup-plugin-esbuild": "^4.7.2",
-    "rollup-plugin-peer-deps-external": "^2.2.2",
     "rollup-plugin-postcss": "^4.0.0",
     "rollup-pluginutils": "^2.8.2",
     "run-script-webpack-plugin": "^0.0.11",

packages/cli/src/lib/builder/config.test.ts

@@ -0,0 +1,59 @@
+/*
+ * Copyright 2020 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 { ExternalOption } from 'rollup';
+import { makeRollupConfigs } from './config';
+import { Output } from './types';
+
+describe('makeRollupConfigs', () => {
+  it('should mark external modules correctly', async () => {
+    const importerPath = '/some/path.ts'; // when specified we don't care about the path
+
+    const [config] = await makeRollupConfigs({
+      outputs: new Set([Output.cjs]),
+    });
+    const external = config.external as Exclude<
+      ExternalOption,
+      string | RegExp | (string | RegExp)[]
+    >;
+
+    expect(external('foo', importerPath, false)).toBe(true);
+    expect(external('./foo', importerPath, false)).toBe(false);
+    expect(external('/foo', importerPath, false)).toBe(false);
+    expect(external('.\\foo', importerPath, false)).toBe(false);
+    expect(external('c:\\foo', importerPath, false)).toBe(false);
+    expect(external('@foo/bar', importerPath, false)).toBe(true);
+    expect(external('../foo', importerPath, false)).toBe(false);
+
+    // Modules without an importer are entry points, i.e. not external
+    expect(external('foo', undefined, false)).toBe(false);
+    expect(external('./foo', undefined, false)).toBe(false);
+    expect(external('/foo', undefined, false)).toBe(false);
+    expect(external('.\\foo', undefined, false)).toBe(false);
+    expect(external('c:\\foo', undefined, false)).toBe(false);
+    expect(external('@foo/bar', undefined, false)).toBe(false);
+    expect(external('../foo', undefined, false)).toBe(false);
+
+    // After modules have been resolved they're never marked as external
+    expect(external('foo', importerPath, true)).toBe(false);
+    expect(external('./foo', importerPath, true)).toBe(false);
+    expect(external('/foo', importerPath, true)).toBe(false);
+    expect(external('.\\foo', importerPath, true)).toBe(false);
+    expect(external('c:\\foo', importerPath, true)).toBe(false);
+    expect(external('@foo/bar', importerPath, true)).toBe(false);
+    expect(external('../foo', importerPath, true)).toBe(false);
+  });
+});

packages/cli/src/lib/builder/config.ts

@@ -17,7 +17,6 @@
 import chalk from 'chalk';
 import fs from 'fs-extra';
 import { relative as relativePath, resolve as resolvePath } from 'path';
-import peerDepsExternal from 'rollup-plugin-peer-deps-external';
 import commonjs from '@rollup/plugin-commonjs';
 import resolve from '@rollup/plugin-node-resolve';
 import postcss from 'rollup-plugin-postcss';
@@ -33,6 +32,19 @@ import { BuildOptions, Output } from './types';
 import { paths } from '../paths';
 import { svgrTemplate } from '../svgrTemplate';
 
+function isFileImport(source: string) {
+  if (source.startsWith('.')) {
+    return true;
+  }
+  if (source.startsWith('/')) {
+    return true;
+  }
+  if (source.match(/[a-z]:/i)) {
+    return true;
+  }
+  return false;
+}
+
 export async function makeRollupConfigs(
   options: BuildOptions,
 ): Promise<RollupOptions[]> {
@@ -81,12 +93,10 @@ export async function makeRollupConfigs(
       output,
       onwarn,
       preserveEntrySignatures: 'strict',
-      external: require('module').builtinModules,
+      // All module imports are always marked as external
+      external: (source, importer, isResolved) =>
+        Boolean(importer && !isResolved && !isFileImport(source)),
       plugins: [
-        peerDepsExternal({
-          packageJsonPath: resolvePath(targetDir, 'package.json'),
-          includeDependencies: true,
-        }),
         resolve({ mainFields }),
         commonjs({
           include: /node_modules/,

yarn.lock

@@ -22048,11 +22048,6 @@ rollup-plugin-esbuild@^4.7.2:
     joycon "^3.0.1"
     jsonc-parser "^3.0.0"
 
-rollup-plugin-peer-deps-external@^2.2.2:
-  version "2.2.4"
-  resolved "https://registry.npmjs.org/rollup-plugin-peer-deps-external/-/rollup-plugin-peer-deps-external-2.2.4.tgz#8a420bbfd6dccc30aeb68c9bf57011f2f109570d"
-  integrity sha512-AWdukIM1+k5JDdAqV/Cxd+nejvno2FVLVeZ74NKggm3Q5s9cbbcOgUPGdbxPi4BXu7xGaZ8HG12F+thImYu/0g==
-
 rollup-plugin-postcss@*, rollup-plugin-postcss@^4.0.0:
   version "4.0.2"
   resolved "https://registry.npmjs.org/rollup-plugin-postcss/-/rollup-plugin-postcss-4.0.2.tgz#15e9462f39475059b368ce0e49c800fa4b1f7050"