fix: make sure all dependencies matches peerdependency range before s…

…kipping it
christophehurpeau · Mar 13, 2023 · 6b0db57 · 6b0db57
1 parent 0093237
commit 6b0db57
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 15 deletions.
diff --git a/dist/definitions/checks/checkPeerDependencies.d.ts.map b/dist/definitions/checks/checkPeerDependencies.d.ts.map
diff --git a/dist/index-node16.mjs b/dist/index-node16.mjs
@@ -125,12 +125,16 @@ function checkPeerDependencies(pkg, reportError, type, allowedPeerIn, providedDe
       if (peerDependenciesMetaPeerDep?.optional) {
         continue;
       }
-
+      let additionalDetails = '';
       // satisfied by another direct dependency
-      if (providedDependencies.some(([depName, depRange]) => depName === peerDepName && semver.intersects(range, depRange))) {
-        continue;
+      const providedDependenciesForDepName = providedDependencies.filter(([depName]) => depName === peerDepName);
+      if (providedDependenciesForDepName.length > 0) {
+        if (providedDependenciesForDepName.every(([, depRange]) => semver.intersects(range, depRange))) {
+          continue;
+        }
+        additionalDetails += ' (required as some dependencies have non-satisfying range too)';
       }
-      reportError(`Missing "${peerDepName}" peer dependency from "${depPkg.name}" in ${type}`, `it should satisfies "${range}" and be in ${allowedPeerIn.join(' or ')}`, missingOnlyWarnsForCheck.shouldWarnsFor(peerDepName));
+      reportError(`Missing "${peerDepName}" peer dependency from "${depPkg.name}" in ${type}`, `it should satisfies "${range}" and be in ${allowedPeerIn.join(' or ')}${additionalDetails}`, missingOnlyWarnsForCheck.shouldWarnsFor(peerDepName));
     } else {
       const versions = versionsIn.map(versionsInType => pkg[versionsInType][peerDepName]);
       versions.forEach((version, index) => {
@@ -173,7 +177,7 @@ async function checkDirectPeerDependencies(isLibrary, pkg, pkgPathName, getDepen
         type: depType,
         pkg: depPkg
       });
-      if (depPkg.dependencies) {
+      if (depPkg.dependencies && !isLibrary) {
         allDirectDependenciesDependencies.push(...Object.entries(depPkg.dependencies));
       }
     }

diff --git a/dist/index-node16.mjs.map b/dist/index-node16.mjs.map
diff --git a/src/checks/checkDirectPeerDependencies.test.ts b/src/checks/checkDirectPeerDependencies.test.ts
@@ -305,4 +305,41 @@ describe('checkDirectPeerDependencies', () => {
     );
     expect(mockReportError).not.toHaveBeenCalled();
   });
+
+  it('should report error when peer dependency is provided by multiple dependencies including non-satisfying range', async () => {
+    await checkDirectPeerDependencies(
+      false,
+      {
+        name: 'test',
+        dependencies: {
+          'some-lib-using-rollup': '1.0.0',
+          'some-lib-providing-rollup-1': '1.0.0',
+          'some-lib-providing-rollup-2': '1.0.0',
+        },
+      },
+      'path',
+      jest
+        .fn()
+        .mockImplementationOnce(() => ({
+          name: 'some-lib-using-rollup',
+          peerDependencies: { rollup: '^1.0.0' },
+        }))
+        .mockImplementationOnce(() => ({
+          name: 'some-lib-providing-rollup-1',
+          dependencies: { rollup: '^1.0.0' },
+        }))
+        .mockImplementationOnce(() => ({
+          name: 'some-lib-providing-rollup-2',
+          dependencies: { rollup: '^2.0.0' },
+        })),
+      createOnlyWarnsForMappingCheck('test', []),
+      createOnlyWarnsForMappingCheck('test', []),
+      createReportError,
+    );
+    expect(mockReportError).toHaveBeenCalledWith(
+      'Missing "rollup" peer dependency from "some-lib-using-rollup" in dependencies',
+      'it should satisfies "^1.0.0" and be in devDependencies or dependencies (required as some dependencies have non-satisfying range too)',
+      false,
+    );
+  });
 });
diff --git a/src/checks/checkPeerDependencies.ts b/src/checks/checkPeerDependencies.ts
@@ -31,21 +31,29 @@ export function checkPeerDependencies(
         continue;
       }
 
+      let additionalDetails = '';
       // satisfied by another direct dependency
-      if (
-        providedDependencies.some(
-          ([depName, depRange]) =>
-            depName === peerDepName && semver.intersects(range, depRange),
-        )
-      ) {
-        continue;
+      const providedDependenciesForDepName = providedDependencies.filter(
+        ([depName]) => depName === peerDepName,
+      );
+      if (providedDependenciesForDepName.length > 0) {
+        if (
+          providedDependenciesForDepName.every(([, depRange]) =>
+            semver.intersects(range, depRange),
+          )
+        ) {
+          continue;
+        }
+
+        additionalDetails +=
+          ' (required as some dependencies have non-satisfying range too)';
       }
 
       reportError(
         `Missing "${peerDepName}" peer dependency from "${depPkg.name}" in ${type}`,
         `it should satisfies "${range}" and be in ${allowedPeerIn.join(
           ' or ',
-        )}`,
+        )}${additionalDetails}`,
         missingOnlyWarnsForCheck.shouldWarnsFor(peerDepName),
       );
     } else {