Merge pull request #1542 from embroider-build/peer-deps-check

Refuse to accept v1 addons as invalid peerDeps

packages/compat/src/build-compat-addon.ts

@@ -3,17 +3,13 @@ import { Package } from '@embroider/core';
 import SmooshPackageJSON from './smoosh-package-json';
 import broccoliMergeTrees from 'broccoli-merge-trees';
 import { Node } from 'broccoli-node-api';
-import buildFunnel from 'broccoli-funnel';
-import { UnwatchedDir, WatchedDir } from 'broccoli-source';
 import EmptyPackageTree from './empty-package-tree';
 
 export default function buildCompatAddon(originalPackage: Package, v1Cache: V1InstanceCache): Node {
   if (originalPackage.isV2Addon()) {
-    // this case is needed when a native-v2 addon depends on a
-    // non-native-v2 addon. (The non-native one will get rewritten and
-    // therefore moved, so to continue depending on it the native one needs to
-    // move too.)
-    return withoutNodeModules(originalPackage);
+    throw new Error(
+      `bug in @embroider/compat. We should not see any v2 addons here, but ${originalPackage.name} as ${originalPackage.root} is a v2 addon`
+    );
   }
 
   let oldPackages = v1Cache.getAddons(originalPackage.root);
@@ -46,10 +42,3 @@ export default function buildCompatAddon(originalPackage: Package, v1Cache: V1In
     return oldPackages[0].v2Tree;
   }
 }
-
-function withoutNodeModules(originalPackage: Package): Node {
-  let Klass = originalPackage.mayRebuild ? WatchedDir : UnwatchedDir;
-  return buildFunnel(new Klass(originalPackage.root), {
-    exclude: ['node_modules', '*/node_modules'],
-  });
-}

packages/compat/src/standalone-addon-build.ts

@@ -1,4 +1,10 @@
-import { Package, PackageCache, RewrittenPackageIndex } from '@embroider/core';
+import {
+  Package,
+  PackageCache,
+  RewrittenPackageIndex,
+  summarizePeerDepViolations,
+  validatePeerDependencies,
+} from '@embroider/core';
 import V1InstanceCache from './v1-instance-cache';
 import buildCompatAddon from './build-compat-addon';
 import { Funnel } from 'broccoli-funnel';
@@ -14,6 +20,25 @@ export function convertLegacyAddons(compatApp: CompatApp) {
   let instanceCache = new V1InstanceCache(compatApp, packageCache);
 
   let appPackage = compatApp.appPackage();
+
+  let violations = validatePeerDependencies(appPackage).filter(({ dep }) => dep.isEmberPackage() && !dep.isV2Ember());
+  if (violations.length > 0) {
+    if (process.env.I_HAVE_BAD_PEER_DEPS_AND_WANT_A_BROKEN_BUILD) {
+      console.warn(
+        `You have set process.env.I_HAVE_BAD_PEER_DEPS_AND_WANT_A_BROKEN_BUILD, so we're ignoring your broken peer deps. Please don't bother reporting any Embroider bugs until you unset it.\n${summarizePeerDepViolations(
+          violations
+        )}`
+      );
+    } else {
+      throw new Error(
+        `Some V1 ember addons are resolving as incorrect peer dependencies. This makes it impossible for us to safely convert them to v2 format.
+See https://github.com/embroider-build/embroider/blob/main/docs/peer-dependency-resolution-issues.md for an explanation of the problem and suggestions for fixing it.
+
+${summarizePeerDepViolations(violations)}`
+      );
+    }
+  }
+
   let v1Addons = findV1Addons(appPackage);
   let index = buildAddonIndex(compatApp, appPackage, v1Addons);
 

packages/shared-internals/src/dep-validation.ts

@@ -0,0 +1,108 @@
+import Package from './package';
+
+// For each package in the graph, discover all the paths to reach that package.
+// That is, we're identifying all its consumers.
+export function crawlDeps(startingPackage: Package): Map<Package, Package[][]> {
+  let queue: { pkg: Package; path: Package[] }[] = [{ pkg: startingPackage, path: [] }];
+  let seen = new Set<Package>();
+  let results = new Map<Package, Package[][]>();
+  for (;;) {
+    let entry = queue.shift();
+    if (!entry) {
+      break;
+    }
+    let { pkg, path } = entry;
+
+    let paths = results.get(pkg);
+    if (paths) {
+      paths.push(path);
+    } else {
+      results.set(pkg, [path]);
+    }
+
+    if (!seen.has(pkg)) {
+      seen.add(pkg);
+      for (let dep of pkg.dependencies) {
+        if (pkg.categorizeDependency(dep.name) !== 'peerDependencies') {
+          queue.push({ pkg: dep, path: [...path, pkg] });
+        }
+      }
+    }
+  }
+  return results;
+}
+
+interface PeerDepViolation {
+  // this package...
+  pkg: Package;
+  // sees this peer dep.
+  dep: Package;
+  // pkg was reached by this path of ancestors
+  ancestors: Package[];
+  // this particular ancestor...
+  ancestor: Package;
+  // provides a conflicting value for the peerDep
+  ancestorsDep: Package;
+}
+
+export function validatePeerDependencies(appPackage: Package): PeerDepViolation[] {
+  let violations = [];
+  for (let [pkg, consumptions] of crawlDeps(appPackage)) {
+    for (let dep of pkg.dependencies) {
+      if (pkg.categorizeDependency(dep.name) === 'peerDependencies') {
+        for (let ancestors of consumptions) {
+          for (let ancestor of ancestors.slice().reverse()) {
+            if (ancestor.hasDependency(dep.name)) {
+              let ancestorsDep = ancestor.dependencies.find(d => d.name === dep.name)!;
+              if (ancestorsDep !== dep && dep.isEmberPackage()) {
+                violations.push({ pkg, dep, ancestors, ancestor, ancestorsDep });
+              }
+              continue;
+            }
+          }
+        }
+      }
+    }
+  }
+  return violations;
+}
+
+export function summarizePeerDepViolations(violations: PeerDepViolation[]): string {
+  let message = [];
+  for (let { pkg, dep, ancestors, ancestor, ancestorsDep } of violations) {
+    for (let [index, a] of ancestors.entries()) {
+      message.push(packageIdSummary(a));
+      if (index + 1 < ancestors.length) {
+        message.push(displayConnection(a, ancestors[index + 1]));
+      } else {
+        message.push(displayConnection(a, pkg));
+      }
+    }
+    message.push(packageIdSummary(pkg));
+    message.push('\n');
+    message.push(`    sees peerDep ${packageIdSummary(dep)}\n      at ${dep.root}\n`);
+    message.push(
+      `    but ${packageIdSummary(ancestor)} is using ${packageIdSummary(ancestorsDep)}\n      at ${
+        ancestorsDep.root
+      }\n\n`
+    );
+  }
+  return message.join('');
+}
+
+function displayConnection(left: Package, right: Package) {
+  if (left.packageJSON.dependencies?.[right.name]) {
+    return ' -> ';
+  }
+  if (left.packageJSON.peerDependencies?.[right.name]) {
+    return ' (peer)-> ';
+  }
+  if (left.packageJSON.devDependencies?.[right.name]) {
+    return ' (dev)-> ';
+  }
+  return ' (?)-> ';
+}
+
+function packageIdSummary(pkg: Package): string {
+  return `${pkg.name}@${pkg.version}`;
+}

packages/shared-internals/src/index.ts

@@ -23,3 +23,5 @@ export {
   version as cacheBustingPluginVersion,
 } from './babel-plugin-cache-busting';
 export { locateEmbroiderWorkingDir } from './working-dir';
+
+export * from './dep-validation';