From 6a1fe077adddb347db12c4c31bafa1ba18b97938 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 4 Mar 2022 13:20:32 +0100 Subject: [PATCH] cli: switch rollup config to consider all package modules external Signed-off-by: Patrik Oldsberg --- .changeset/tender-berries-lie.md | 5 ++ docs/local-dev/cli-build-system.md | 10 ++-- packages/cli/package.json | 1 - packages/cli/src/lib/builder/config.test.ts | 59 +++++++++++++++++++++ packages/cli/src/lib/builder/config.ts | 22 +++++--- yarn.lock | 5 -- 6 files changed, 84 insertions(+), 18 deletions(-) create mode 100644 .changeset/tender-berries-lie.md create mode 100644 packages/cli/src/lib/builder/config.test.ts diff --git a/.changeset/tender-berries-lie.md b/.changeset/tender-berries-lie.md new file mode 100644 index 0000000000..74a3eb4707 --- /dev/null +++ b/.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. diff --git a/docs/local-dev/cli-build-system.md b/docs/local-dev/cli-build-system.md index f57efc1255..aed3ba70bc 100644 --- a/docs/local-dev/cli-build-system.md +++ b/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 diff --git a/packages/cli/package.json b/packages/cli/package.json index af2c3afb2d..ec158fa5c1 100644 --- a/packages/cli/package.json +++ b/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", diff --git a/packages/cli/src/lib/builder/config.test.ts b/packages/cli/src/lib/builder/config.test.ts new file mode 100644 index 0000000000..a2e99da73b --- /dev/null +++ b/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); + }); +}); diff --git a/packages/cli/src/lib/builder/config.ts b/packages/cli/src/lib/builder/config.ts index 0a214f1a7a..fe91cedbb0 100644 --- a/packages/cli/src/lib/builder/config.ts +++ b/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 { @@ -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/, diff --git a/yarn.lock b/yarn.lock index 5d8b1c24d7..02532c5286 100644 --- a/yarn.lock +++ b/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"