diff --git a/.changeset/swift-sheep-ring.md b/.changeset/swift-sheep-ring.md new file mode 100644 index 0000000000..f4cf92c29d --- /dev/null +++ b/.changeset/swift-sheep-ring.md @@ -0,0 +1,5 @@ +--- +'@backstage/cli': patch +--- + +Re-enable symlink resolution during bundling, and switch to using a resolve plugin for external linked packages. diff --git a/packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.test.ts b/packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.test.ts new file mode 100644 index 0000000000..53512567eb --- /dev/null +++ b/packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.test.ts @@ -0,0 +1,140 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { LinkedPackageResolvePlugin } from './LinkedPackageResolvePlugin'; + +describe('LinkedPackageResolvePlugin', () => { + it('should re-write paths for external packages', () => { + const plugin = new LinkedPackageResolvePlugin('/root/repo/node_modules', [ + { + name: 'a', + location: '/root/external-a', + }, + { + name: '@s/b', + location: '/root/external-b', + }, + ]); + + const tapAsync = jest.fn(); + const doResolve = jest.fn(); + + const resolver = { + hooks: { resolve: { tapAsync } }, + doResolve, + }; + plugin.apply(resolver); + + expect(tapAsync).toHaveBeenCalledTimes(1); + expect(tapAsync).toHaveBeenCalledWith( + 'LinkedPackageResolvePlugin', + expect.any(Function), + ); + expect(doResolve).toHaveBeenCalledTimes(0); + + // Internal module resolution is not affected + const tap = tapAsync.mock.calls[0][1]; + const callbackX = jest.fn(); + tap( + { + request: '/root/repo/package/x/src/module.ts', + path: '/root/repo/package/x/src', + context: { + issuer: '/root/repo/package/x/src/index.ts', + }, + }, + 'some-context', + callbackX, + ); + expect(callbackX).toHaveBeenCalledTimes(1); + expect(callbackX).toHaveBeenCalledWith(); + expect(doResolve).toHaveBeenCalledTimes(0); + + // Path is sometimes false + const callbackFalse = jest.fn(); + tap( + { + request: 'dummy', + path: false, + }, + 'some-context', + callbackFalse, + ); + expect(callbackFalse).toHaveBeenCalledTimes(1); + expect(callbackFalse).toHaveBeenCalledWith(); + expect(doResolve).toHaveBeenCalledTimes(0); + + // External modules have their path and issuer context rewritten, but not the request + const callbackA = jest.fn(); + tap( + { + request: '/root/external-a/src/module.ts', + path: '/root/external-a/src', + context: { + issuer: '/root/external-a/src/index.ts', + }, + }, + 'some-context', + callbackA, + ); + expect(callbackA).toHaveBeenCalledTimes(0); + expect(doResolve).toHaveBeenCalledTimes(1); + expect(doResolve).toHaveBeenCalledWith( + resolver.hooks.resolve, + { + request: '/root/external-a/src/module.ts', + path: '/root/repo/node_modules/a/src', + context: { + issuer: '/root/repo/node_modules/a/src/index.ts', + }, + }, + 'resolve /root/external-a/src/module.ts in /root/repo/node_modules/a', + 'some-context', + callbackA, + ); + + // Also handles scoped packages correctly, and issuer is not required + const callbackB = jest.fn(); + tap( + { + request: '/root/external-b/src/module.ts', + path: '/root/external-b/src', + context: { + issuer: false, + }, + }, + 'some-context', + callbackB, + ); + expect(callbackB).toHaveBeenCalledTimes(0); + expect(doResolve).toHaveBeenCalledTimes(2); + expect(doResolve).toHaveBeenLastCalledWith( + resolver.hooks.resolve, + { + request: '/root/external-b/src/module.ts', + path: '/root/repo/node_modules/@s/b/src', + context: { + issuer: false, + }, + }, + 'resolve /root/external-b/src/module.ts in /root/repo/node_modules/@s/b', + 'some-context', + callbackB, + ); + + expect(tapAsync).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.ts b/packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.ts new file mode 100644 index 0000000000..86269a5828 --- /dev/null +++ b/packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.ts @@ -0,0 +1,81 @@ +/* + * Copyright 2020 Spotify AB + * + * 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 { resolve as resolvePath } from 'path'; +import { ResolvePlugin } from 'webpack'; +import { LernaPackage } from './types'; + +// Enables proper resolution of packages when linking in external packages. +// Without this the packages would depend on dependencies in the node_modules +// of the external packages themselves, leading to module duplication +export class LinkedPackageResolvePlugin implements ResolvePlugin { + constructor( + private readonly targetModules: string, + private readonly packages: LernaPackage[], + ) {} + + apply(resolver: any) { + resolver.hooks.resolve.tapAsync( + 'LinkedPackageResolvePlugin', + ( + data: { + request: string; + path?: false | string; + context?: { issuer?: string }; + }, + context: unknown, + callback: () => void, + ) => { + const pkg = this.packages.find( + pkg => data.path && data.path.startsWith(pkg.location), + ); + if (!pkg) { + callback(); + return; + } + + // pkg here is an external package. We rewrite the context of any imports to resolve + // from the location of the package within the node_modules of the target root rather + // than the real location of the external package. + const modulesLocation = resolvePath(this.targetModules, pkg.name); + const newContext = data.context?.issuer + ? { + ...data.context, + issuer: data.context.issuer.replace( + pkg.location, + modulesLocation, + ), + } + : data.context; + + // Re-run resolution but this time from the point of view of our target monorepo rather + // than the location of the external package. By resolving modules using this method we avoid + // pulling in e.g. `react` from the external repo, which would otherwise lead to conflicts. + resolver.doResolve( + resolver.hooks.resolve, + { + ...data, + context: newContext, + path: data.path && data.path.replace(pkg.location, modulesLocation), + }, + `resolve ${data.request} in ${modulesLocation}`, + context, + callback, + ); + }, + ); + } +} diff --git a/packages/cli/src/lib/bundler/config.ts b/packages/cli/src/lib/bundler/config.ts index b5a27dd624..3c336401ee 100644 --- a/packages/cli/src/lib/bundler/config.ts +++ b/packages/cli/src/lib/bundler/config.ts @@ -26,7 +26,8 @@ import { optimization } from './optimization'; import { Config } from '@backstage/config'; import { BundlingPaths } from './paths'; import { transforms } from './transforms'; -import { BundlingOptions, BackendBundlingOptions } from './types'; +import { LinkedPackageResolvePlugin } from './LinkedPackageResolvePlugin'; +import { BundlingOptions, BackendBundlingOptions, LernaPackage } from './types'; import { version } from '../../lib/version'; import { paths as cliPaths } from '../../lib/paths'; import { runPlain } from '../run'; @@ -70,9 +71,7 @@ async function readBuildInfo() { }; } -async function loadLernaPackages(): Promise< - { name: string; location: string }[] -> { +async function loadLernaPackages(): Promise { const LernaProject = require('@lerna/project'); const project = new LernaProject(cliPaths.targetDir); return project.getPackages(); @@ -85,12 +84,10 @@ export async function createConfig( const { checksEnabled, isDev, frontendConfig } = options; const packages = await loadLernaPackages(); - const { plugins, loaders } = transforms({ - ...options, - externalTransforms: packages.map(({ name }) => - cliPaths.resolveTargetRoot('node_modules', name), - ), - }); + const { plugins, loaders } = transforms(options); + // Any package that is part of the monorepo but outside the monorepo root dir need + // separate resolution logic. + const externalPkgs = packages.filter(p => !p.location.startsWith(paths.root)); const baseUrl = frontendConfig.getString('app.baseUrl'); const validBaseUrl = new URL(baseUrl); @@ -165,6 +162,7 @@ export async function createConfig( extensions: ['.ts', '.tsx', '.mjs', '.js', '.jsx'], mainFields: ['browser', 'module', 'main'], plugins: [ + new LinkedPackageResolvePlugin(paths.rootNodeModules, externalPkgs), new ModuleScopePlugin( [paths.targetSrc, paths.targetDev], [paths.targetPackageJson], @@ -173,10 +171,6 @@ export async function createConfig( alias: { 'react-dom': '@hot-loader/react-dom', }, - // Enables proper resolution of packages when linking in external packages. - // Without this the packages would depend on dependencies in the node_modules - // of the external packages themselves, leading to module duplication - symlinks: false, }, module: { rules: loaders, @@ -205,13 +199,9 @@ export async function createBackendConfig( const moduleDirs = packages.map((p: any) => resolvePath(p.location, 'node_modules'), ); + const externalPkgs = packages.filter(p => !p.location.startsWith(paths.root)); // See frontend config - const { loaders } = transforms({ - ...options, - externalTransforms: packages.map(({ name }) => - cliPaths.resolveTargetRoot('node_modules', name), - ), - }); + const { loaders } = transforms(options); return { mode: isDev ? 'development' : 'production', @@ -253,6 +243,7 @@ export async function createBackendConfig( mainFields: ['browser', 'module', 'main'], modules: [paths.rootNodeModules, ...moduleDirs], plugins: [ + new LinkedPackageResolvePlugin(paths.rootNodeModules, externalPkgs), new ModuleScopePlugin( [paths.targetSrc, paths.targetDev], [paths.targetPackageJson], @@ -261,7 +252,6 @@ export async function createBackendConfig( alias: { 'react-dom': '@hot-loader/react-dom', }, - symlinks: false, // See frontend config, added here for the same reason }, module: { rules: loaders, diff --git a/packages/cli/src/lib/bundler/transforms.ts b/packages/cli/src/lib/bundler/transforms.ts index 19f1acff77..4f097343c2 100644 --- a/packages/cli/src/lib/bundler/transforms.ts +++ b/packages/cli/src/lib/bundler/transforms.ts @@ -25,28 +25,17 @@ type Transforms = { type TransformOptions = { isDev: boolean; - // External paths that should be transformed - externalTransforms: string[]; }; export const transforms = (options: TransformOptions): Transforms => { - const { isDev, externalTransforms } = options; + const { isDev } = options; const extraTransforms = isDev ? ['react-hot-loader'] : []; - const transformExcludeCondition = { - or: [ - // This makes sure we don't transform node_modules inside any of the local monorepo packages - /node_modules.*node_modules/, - // This excludes the local monorepo packages from the excludes, meaning they will be transformed - { and: [/node_modules/, { not: externalTransforms }] }, - ], - }; - const loaders = [ { test: /\.(tsx?)$/, - exclude: transformExcludeCondition, + exclude: /node_modules/, loader: require.resolve('@sucrase/webpack-loader'), options: { transforms: ['typescript', 'jsx', ...extraTransforms], @@ -55,7 +44,7 @@ export const transforms = (options: TransformOptions): Transforms => { }, { test: /\.(jsx?|mjs)$/, - exclude: transformExcludeCondition, + exclude: /node_modules/, loader: require.resolve('@sucrase/webpack-loader'), options: { transforms: ['jsx', ...extraTransforms], diff --git a/packages/cli/src/lib/bundler/types.ts b/packages/cli/src/lib/bundler/types.ts index 71343a1761..3bd941dd0f 100644 --- a/packages/cli/src/lib/bundler/types.ts +++ b/packages/cli/src/lib/bundler/types.ts @@ -53,3 +53,8 @@ export type BackendServeOptions = BundlingPathsOptions & { checksEnabled: boolean; inspectEnabled: boolean; }; + +export type LernaPackage = { + name: string; + location: string; +};