Skip to content

Commit

Permalink
fix(resolve): makes dependency classifier handle more flavours of the…
Browse files Browse the repository at this point in the history
… 'workspaces' type alias field (#920)

## 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)
  • Loading branch information
sverweij committed Mar 16, 2024
1 parent 588af8d commit 1c2920e
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 4 deletions.
21 changes: 17 additions & 4 deletions src/extract/resolve/module-classifiers.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
//
Expand All @@ -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;
Expand Down
55 changes: 55 additions & 0 deletions test/extract/resolve/module-classifiers.get-alias-types.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit 1c2920e

Please sign in to comment.