Repository: sverweij/dependency-cruiser (js)

Issue:
Issue: using `workspace:` to implement nohoist in a package.json while using pnpm throws an error
<!--- Provide a general summary of the issue in the Title above -->
If you are using pnpm to do workspace manage, you don't define your workspaces in package.json - `workspaces` property, instead, separately in a pnpm-workspaces.yaml file. 

One of our packages had a leftover workspaces nohoist property from when we used to use yarn, like this:

```
 "workspaces": {
    "nohoist": [
      "vite",
      "rollup"
    ]
  },
 ```
 
 but doesn't specify any workspaces. 
 
 When analyzed with `pnpm depcruise packages/package-name -T json -c > results.json
 
 it throws an error: 
 
 ```
   ERROR: Extracting dependencies ran afoul of...

  pWorkspace.endsWith is not a function
... in packages/...
```

Removing the nohoist makes it work as expected. 

### Expected Behavior

If the workspaces property of the package.json is an object with only a nohoist property, ignore it. 

<!--- Tell us what you expect should happen -->

### Current Behavior
 ```
   ERROR: Extracting dependencies ran afoul of...

  pWorkspace.endsWith is not a function
... in packages/...
```

<!--- Tell what happens instead -->

### Possible Solution

If the workspaces property of the package.json is an object with only a nohoist property, ignore it in 

<!--- Not obligatory, but suggestions how to fix the bug, or ideas what the reason for the bug is are welcome. -->

### Steps to Reproduce (for bugs)

<!--- Provide a link to a live example, or an unambiguous set of steps to -->
<!--- reproduce this bug. Include code to reproduce, if relevant -->

1.
2.
3.
4.

### Context

<!--- How has this issue affected you? What are you trying to accomplish? -->
<!--- Providing context helps us understand better, which will likely result in better, faster, stronger fixes -->

### Your Environment

<!--- Include as many relevant details about the environment you experienced the bug in -->

- Version used:
- Node version:
- Operating System and version:
- Link to your project:


Pull request:
fix(resolve): makes dependency classifier handle more flavours of the 'workspaces' type alias field
## Description

- ignores 'workspaces' field when it isn't an array _and_  it doesn't contain a yarn1 workspaces.packages field
- also classifies dependencies resolving via yarn1 workspaces.packages fields  as _aliased_, _aliased-workspace_

## Motivation and Context

fixes #919 

## How Has This Been Tested?

- [x] green ci
- [x] additional automated non-regression test

Note: a version with this fix has been published as `dependency-cruiser@16.2.4-beta-1` - once the PR is merged it'll be part of dependency-cruiser@16.2.4.

## 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).


Patch:
diff --git a/src/extract/resolve/module-classifiers.mjs b/src/extract/resolve/module-classifiers.mjs
index e9f5c08d..4fe75236 100644
--- a/src/extract/resolve/module-classifiers.mjs
+++ b/src/extract/resolve/module-classifiers.mjs
@@ -122,6 +122,16 @@ function isWebPackAliased(pModuleName, pAliasObject) {
   );
 }
 
+function getWorkspacesArray(pManifestWorkspacesField) {
+  if (Array.isArray(pManifestWorkspacesField)) {
+    return pManifestWorkspacesField;
+  }
+  if (pManifestWorkspacesField?.packages) {
+    return pManifestWorkspacesField.packages;
+  }
+  return [];
+}
+
 /**
  * @param {string} pModuleName
  * @param {string} pResolvedModuleName
@@ -131,7 +141,11 @@ function isWebPackAliased(pModuleName, pAliasObject) {
 // eslint-disable-next-line max-lines-per-function
 function isWorkspaceAliased(pModuleName, pResolvedModuleName, pManifest) {
   // reference: https://docs.npmjs.com/cli/v10/using-npm/workspaces
-  if (pManifest?.workspaces) {
+  // for yarn the workspaces field might be either an array or
+  // an object. To prevent the code from borking we check whether it's an array
+  // see https://github.com/sverweij/dependency-cruiser/issues/919
+  const lWorkspaces = getWorkspacesArray(pManifest?.workspaces);
+  if (lWorkspaces.length >= 0) {
     // workspaces are an array of globs that match the (sub) workspace
     // folder itself only.
     //
@@ -154,9 +168,8 @@ 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.
-    const lModuleFriendlyWorkspaceGlobs = pManifest.workspaces.map(
-      (pWorkspace) =>
-        pWorkspace.endsWith("/") ? `${pWorkspace}**` : `${pWorkspace}/**`,
+    const lModuleFriendlyWorkspaceGlobs = lWorkspaces.map((pWorkspace) =>
+      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 09dea134..52ca0157 100644
--- a/test/extract/resolve/module-classifiers.get-alias-types.spec.mjs
+++ b/test/extract/resolve/module-classifiers.get-alias-types.spec.mjs
@@ -219,6 +219,61 @@ describe("[I] extract/resolve/module-classifiers - getAliasTypes", () => {
     );
   });
 
+  it("ignores workspaces in package.json if it's not an array and the object doesn't contain a packages field", () => {
+    const lManifest = {
+      name: "test",
+      version: "1.0.0",
+      dependencies: {},
+      workspaces: {
+        // yarn(1) specific
+        nohoist: ["packages/", "foo", "bar"],
+      },
+    };
+    const lResolveOptions = {
+      baseDirectory: "over/the/rainbow",
+      alias: {
+        "@": "./src",
+      },
+    };
+    deepEqual(
+      getAliasTypes(
+        "some-workspaced-local-package",
+        "packages/a-package/index.js",
+        lResolveOptions,
+        lManifest,
+      ),
+      [],
+    );
+  });
+
+  it("returns aliased and aliased-workspace for workspace alias (for yarn 1 workspaces.packages)", () => {
+    const lManifest = {
+      name: "test",
+      version: "1.0.0",
+      dependencies: {},
+      workspaces: {
+        // yarn(1) specific notation
+        packages: ["packages/"],
+        nohoist: ["foo", "bar"],
+      },
+    };
+    const lResolveOptions = {
+      baseDirectory: "over/the/rainbow",
+      alias: {
+        "@": "./src",
+      },
+    };
+    deepEqual(
+      getAliasTypes(
+        "some-workspaced-local-package",
+        "packages/a-package/index.js",
+        lResolveOptions,
+        lManifest,
+      ),
+      ["aliased", "aliased-workspace"],
+    );
+  });
+
   it("doesn't run aliased and aliased-workspace for when resolved matches a workspace, but module requested is relative", () => {
     const lManifest = {
       name: "test",