From c97bca7d215413da1d41b094d5635dc4915461de Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Sun, 13 Dec 2020 12:30:23 +0100 Subject: [PATCH] cli: separate out LinkedPackageResolvePlugin and adds tests --- .../LinkedPackageResolvePlugin.test.ts | 140 ++++++++++++++++++ .../lib/bundler/LinkedPackageResolvePlugin.ts | 81 ++++++++++ packages/cli/src/lib/bundler/config.ts | 70 +-------- packages/cli/src/lib/bundler/types.ts | 5 + 4 files changed, 229 insertions(+), 67 deletions(-) create mode 100644 packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.test.ts create mode 100644 packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.ts 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 3c15ead0f2..3c336401ee 100644 --- a/packages/cli/src/lib/bundler/config.ts +++ b/packages/cli/src/lib/bundler/config.ts @@ -20,13 +20,14 @@ import ForkTsCheckerWebpackPlugin from 'fork-ts-checker-webpack-plugin'; import HtmlWebpackPlugin from 'html-webpack-plugin'; import ModuleScopePlugin from 'react-dev-utils/ModuleScopePlugin'; import StartServerPlugin from 'start-server-webpack-plugin'; -import webpack, { ResolvePlugin } from 'webpack'; +import webpack from 'webpack'; import nodeExternals from 'webpack-node-externals'; 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,77 +71,12 @@ async function readBuildInfo() { }; } -type LernaPackage = { - name: string; - location: string; -}; - async function loadLernaPackages(): Promise { const LernaProject = require('@lerna/project'); const project = new LernaProject(cliPaths.targetDir); return project.getPackages(); } -// 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 -class LinkedPackageResolvePlugin implements ResolvePlugin { - constructor( - private readonly targetModules: string, - private readonly packages: LernaPackage[], - ) {} - - apply(resolver: any) { - resolver.hooks.resolve.tapAsync( - 'LinkedPackageResolvePlugin', - ( - request: { path?: false | string; context?: { issuer?: string } }, - context: unknown, - callback: () => void, - ) => { - const pkg = this.packages.find( - pkg => request.path && request.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 = request.context?.issuer - ? { - ...request.context, - issuer: request.context.issuer.replace( - pkg.location, - modulesLocation, - ), - } - : request.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, - { - ...request, - context: newContext, - path: - request.path && - request.path.replace(pkg.location, modulesLocation), - }, - null, - context, - callback, - ); - }, - ); - } -} - export async function createConfig( paths: BundlingPaths, options: BundlingOptions, 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; +};