sverweij / dependency-cruiser

Validate and visualize dependencies. Your rules. JavaScript, TypeScript, CoffeeScript. ES6, CommonJS, AMD.
https://npmjs.com/dependency-cruiser
MIT License
5.27k stars 250 forks source link

Issue: using `workspace:` to implement nohoist in a package.json while using pnpm throws an error #919

Closed danroberts closed 7 months ago

danroberts commented 7 months ago

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.

Current Behavior

   ERROR: Extracting dependencies ran afoul of...

  pWorkspace.endsWith is not a function
... in packages/...

Possible Solution

If the workspaces property of the package.json is an object with only a nohoist property, ignore it in

Steps to Reproduce (for bugs)

1. 2. 3. 4.

Context

Your Environment

sverweij commented 7 months ago

Hi @danroberts thanks for raising this (clear & well documented) bug report.

I've created a fix (see linked PR) and published dependency-cruiser@16.2.4-beta-1 (with the beta tag) to npmjs that contains it.

Let me know whether or not that fixes the issue on your side as well!

danroberts commented 7 months ago

Hi @sverweij! Thanks for the quick response. It does solve my problem, however, it is possible that your fix might regress the following example which is valid for yarn:

  "workspaces": {
    "packages": [
      "packages/*"
    ],
    "nohoist": [
      "**/pkgA/**"
    ]
  },

I'm not really sure how the cruiser would handle that but it's possible it might miss the workspaces defined in the packages property.

sverweij commented 7 months ago

Hi @danroberts thanks for checking! That yarn (1) shape of workspaces indeed won't work. It didn't in the previous version either b.t.w., so that'll be a good addition.

It won't (and didn't) lead to missing workspace dependencies (determining that happens elsewhere), only thing that'll happen is that these specific workspace aliases will not get classified as such.

Anyway - PR updated, & published as dependency-cruiser@16.2.4-beta-2 (since a few minutes).

sverweij commented 7 months ago

FYI @danroberts - I've just released dependency-cruiser@16.2.4 that contains the fix for this issue as well.