Repository: sverweij/dependency-cruiser
Pull request #955 (2024-08-31 10:35:20)

fix(extract): ignore entries in 'workspaces' arrays that aren't strings (instead of throwing)
## Description

- ignore entries in 'workspaces' arrays that aren't strings, instead of throwing an exception


Yarn workspaces is an array of strings according to [its documentation](https://yarnpkg.com/configuration/manifest#workspaces). Apparently is possible (and accepted?) to have other types than strings as well. Not sure whether that is _valid_, but at least it's happening in the field, witnessing #947, so we deal with it.

## Motivation and Context

Addresses the error mentioned in #947 (and likely fixes it)

## How Has This Been Tested?

- [x] green ci
- [x] additional automated non-regression tests

## Types of changes

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Documentation only change
- [ ] Refactor (non-breaking change which fixes an issue without changing functionality)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Checklist

- [x] :book:

  - My change doesn't require a documentation update, or ...
  - it _does_ and I have updated it

- [x] :balance_scale:
  - The contribution will be subject to [The MIT license](https://github.com/sverweij/dependency-cruiser/blob/main/LICENSE), and I'm OK with that.
  - The contribution is my own original work.
  - I am ok with the stuff in [**CONTRIBUTING.md**](https://github.com/sverweij/dependency-cruiser/blob/main/.github/CONTRIBUTING.md).


codeclimate[bot]: Code Climate has analyzed commit 76955a6e and detected **0 issues** on this pull request.

View more on [Code Climate](https://codeclimate.com/github/sverweij/dependency-cruiser/pull/955).


Patch:
diff --git a/src/extract/resolve/module-classifiers.mjs b/src/extract/resolve/module-classifiers.mjs
index 6a4c7e45..ee600b78 100644
--- a/src/extract/resolve/module-classifiers.mjs
+++ b/src/extract/resolve/module-classifiers.mjs
@@ -168,8 +168,15 @@ function isWorkspaceAliased(pModuleName, pResolvedModuleName, pManifest) {
     // oh and: ```picomatch.isMatch('asdf', 'asdf/**') === true``` so
     // in case it's only 'asdf' that's in the resolved module name for some reason
     // we're good as well.
+    //
+    // workspaces is supposed to be array of strings, where each string is
+    // a glob pattern. However, in the field there's occasions where it's
+    // not a string. We'll just ignore those for now, hence the presence of
+    // the `typeof pWorkspace === "string"` check.
     const lModuleFriendlyWorkspaceGlobs = lWorkspaces.map((pWorkspace) =>
-      pWorkspace.endsWith("/") ? `${pWorkspace}**` : `${pWorkspace}/**`,
+      typeof pWorkspace === "string" && pWorkspace.endsWith("/")
+        ? `${pWorkspace}**`
+        : `${pWorkspace}/**`,
     );
     if (picomatch.isMatch(pResolvedModuleName, lModuleFriendlyWorkspaceGlobs)) {
       return true;


Test patch:
diff --git a/test/extract/resolve/module-classifiers.get-alias-types.spec.mjs b/test/extract/resolve/module-classifiers.get-alias-types.spec.mjs
index 52ca0157..2dbef29c 100644
--- a/test/extract/resolve/module-classifiers.get-alias-types.spec.mjs
+++ b/test/extract/resolve/module-classifiers.get-alias-types.spec.mjs
@@ -293,6 +293,44 @@ describe("[I] extract/resolve/module-classifiers - getAliasTypes", () => {
     );
   });
 
+  it("doesn't run aliased and aliased-workspace for when workspaces is an array, but the entry isn't a string", () => {
+    const lManifest = {
+      name: "test",
+      version: "1.0.0",
+      dependencies: {},
+      workspaces: [{ "packages/*": "packages/*" }],
+    };
+    const lResolveOptions = {};
+    deepEqual(
+      getAliasTypes(
+        "some-workspaced-local-package",
+        "packages/a-package/index.js",
+        lResolveOptions,
+        lManifest,
+      ),
+      [],
+    );
+  });
+
+  it("skips over entries in the workspaces array that aren't string, but still uses the rest", () => {
+    const lManifest = {
+      name: "test",
+      version: "1.0.0",
+      dependencies: {},
+      workspaces: [{ "packages/*": "packages/*" }, "packages/*"],
+    };
+    const lResolveOptions = {};
+    deepEqual(
+      getAliasTypes(
+        "some-workspaced-local-package",
+        "packages/a-package/index.js",
+        lResolveOptions,
+        lManifest,
+      ),
+      ["aliased", "aliased-workspace"],
+    );
+  });
+
   it("classifies as a webpack alias if it could be both a webpack alias _and_ a workspace alias", () => {
     const lManifest = {
       name: "test",