From 818d45e9422eef85c216ce4c40b0b0905a13768a Mon Sep 17 00:00:00 2001
From: Joel Low <joel.low@grabtaxi.com>
Date: Thu, 7 Jan 2021 17:00:41 +0800
Subject: [PATCH] Fix detection of child directories

Using `string.startsWith` is insufficient when two different directories
share a prefix, e.g. `./packageA` and `./packageAA`.
---
 .changeset/modern-worms-promise.md              |  5 +++++
 .../bundler/LinkedPackageResolvePlugin.test.ts  | 17 +++++++++++++++++
 .../lib/bundler/LinkedPackageResolvePlugin.ts   |  3 ++-
 packages/cli/src/lib/bundler/config.ts          | 10 +++++++---
 packages/cli/src/lib/bundler/paths.ts           | 17 +++++++++++++++++
 5 files changed, 48 insertions(+), 4 deletions(-)
 create mode 100644 .changeset/modern-worms-promise.md

diff --git a/.changeset/modern-worms-promise.md b/.changeset/modern-worms-promise.md
new file mode 100644
index 0000000000..f14ab468fc
--- /dev/null
+++ b/.changeset/modern-worms-promise.md
@@ -0,0 +1,5 @@
+---
+'@backstage/cli': patch
+---
+
+Fix detection of external package child directories
diff --git a/packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.test.ts b/packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.test.ts
index b4580ea281..f3906bfd0d 100644
--- a/packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.test.ts
+++ b/packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.test.ts
@@ -84,6 +84,23 @@ describe('LinkedPackageResolvePlugin', () => {
     expect(callbackFalse).toHaveBeenCalledWith();
     expect(doResolve).toHaveBeenCalledTimes(0);
 
+    // Internal modules with a path prefix of an external module
+    const callbackY = jest.fn();
+    tap(
+      {
+        request: path.resolve(root, 'external-aa/src/module.ts'),
+        path: path.resolve(root, 'external-aa/src'),
+        context: {
+          issuer: path.resolve(root, 'external-aa/src/index.ts'),
+        },
+      },
+      'some-context',
+      callbackY,
+    );
+    expect(callbackY).toHaveBeenCalledTimes(1);
+    expect(callbackY).toHaveBeenCalledWith();
+    expect(doResolve).toHaveBeenCalledTimes(0);
+
     // External modules have their path and issuer context rewritten, but not the request
     const callbackA = jest.fn();
     tap(
diff --git a/packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.ts b/packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.ts
index 86269a5828..0478337c61 100644
--- a/packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.ts
+++ b/packages/cli/src/lib/bundler/LinkedPackageResolvePlugin.ts
@@ -16,6 +16,7 @@
 
 import { resolve as resolvePath } from 'path';
 import { ResolvePlugin } from 'webpack';
+import { isChildPath } from './paths';
 import { LernaPackage } from './types';
 
 // Enables proper resolution of packages when linking in external packages.
@@ -40,7 +41,7 @@ export class LinkedPackageResolvePlugin implements ResolvePlugin {
         callback: () => void,
       ) => {
         const pkg = this.packages.find(
-          pkg => data.path && data.path.startsWith(pkg.location),
+          pkg => data.path && isChildPath(pkg.location, data.path),
         );
         if (!pkg) {
           callback();
diff --git a/packages/cli/src/lib/bundler/config.ts b/packages/cli/src/lib/bundler/config.ts
index 3c336401ee..c53a8329f5 100644
--- a/packages/cli/src/lib/bundler/config.ts
+++ b/packages/cli/src/lib/bundler/config.ts
@@ -24,7 +24,7 @@ import webpack from 'webpack';
 import nodeExternals from 'webpack-node-externals';
 import { optimization } from './optimization';
 import { Config } from '@backstage/config';
-import { BundlingPaths } from './paths';
+import { BundlingPaths, isChildPath } from './paths';
 import { transforms } from './transforms';
 import { LinkedPackageResolvePlugin } from './LinkedPackageResolvePlugin';
 import { BundlingOptions, BackendBundlingOptions, LernaPackage } from './types';
@@ -87,7 +87,9 @@ export async function createConfig(
   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 externalPkgs = packages.filter(
+    p => !isChildPath(paths.root, p.location),
+  );
 
   const baseUrl = frontendConfig.getString('app.baseUrl');
   const validBaseUrl = new URL(baseUrl);
@@ -199,7 +201,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 externalPkgs = packages.filter(
+    p => !isChildPath(paths.root, p.location),
+  ); // See frontend config
 
   const { loaders } = transforms(options);
 
diff --git a/packages/cli/src/lib/bundler/paths.ts b/packages/cli/src/lib/bundler/paths.ts
index 390e41a345..f38416ca4e 100644
--- a/packages/cli/src/lib/bundler/paths.ts
+++ b/packages/cli/src/lib/bundler/paths.ts
@@ -15,8 +15,25 @@
  */
 
 import fs from 'fs-extra';
+import path from 'path';
 import { paths } from '../paths';
 
+/**
+ * Checks if dir is the same as or a child of base.
+ */
+export function isChildPath(base: string, dir: string): boolean {
+  const relativePath = path.relative(base, dir);
+  if (relativePath === '') {
+    // The same directory
+    return true;
+  }
+
+  const outsideBase = relativePath.startsWith('..'); // not outside base
+  const differentDrive = path.isAbsolute(relativePath); // on Windows, this means dir is on a different drive from base.
+
+  return !outsideBase && !differentDrive;
+}
+
 export type BundlingPathsOptions = {
   // bundle entrypoint, e.g. 'src/index'
   entry: string;