Skip to content

Commit

Permalink
feat(extract): makes support for workspaces explicit (#864)
Browse files Browse the repository at this point in the history
## Description

- adds explicit detection of modules aliased via
[workspaces](https://docs.npmjs.com/cli/v10/using-npm/workspaces)
- introduces the `aliased-workspace` dependency type

## Motivation and Context

Imports from workspaces have been well-supported in dependency-cruiser
since the stone age. Not by brilliant foresight, but because it plugs
into the regular node.js module resolution - workspaces are symlinks in
node_modules sorta automatically linked to local folders.
Dependency-cruiser didn't explicitly name them as separate
dependency-types (instead slotting them into dependency type 'local' or
'npm-no-pkg' (when `npm i` wasn't run on the folder yet)). This PR
corrects that oversight.

This PR also fixes a bug uncovered by @throrin19: workspace imports were
erroneously classified as tsconfig aliases (see #863 for details).

## How Has This Been Tested?

- [x] green ci
- [x] additional unit tests
- [x] additional integration test


## Types of changes

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Documentation only change
- [ ] Refactor (non-breaking change which fixes an issue without
changing functionality)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
  • Loading branch information
sverweij authored Nov 12, 2023
1 parent 41b20c6 commit fce008d
Show file tree
Hide file tree
Showing 34 changed files with 668 additions and 17 deletions.
2 changes: 1 addition & 1 deletion .c8rc.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"checkCoverage": true,
"statements": 99.89,
"branches": 98.72,
"branches": 98.76,
"functions": 100,
"lines": 99.89,
"exclude": [
Expand Down
3 changes: 2 additions & 1 deletion doc/rules-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -933,8 +933,9 @@ This is a list of dependency types dependency-cruiser currently detects.
| core | it's a core module | "fs" |
| aliased | the module was imported via an alias - always occurs alongside one of 'aliased-\*' dependency types below _and_ alongside the dependency type of the dependency it's aliased to (so: local, npm, core ,...) | "~/hello.ts" |
| aliased-subpath-import | the module was imported via a [subpath import](https://nodejs.org/api/packages.html#subpath-imports) | "#thing/hello.mjs" |
| aliased-webpack | the module was imported via a [webpack resolve alias](https://webpack.js.org/configuration/resolve/#resolvealias) | "Utilities" |
| aliased-tsconfig | the module was imported via a typescript [compilerOptions.paths setting in tsconfig](https://www.typescriptlang.org/tsconfig#paths) | "@thing/hello" |
| aliased-webpack | the module was imported via a [webpack resolve alias](https://webpack.js.org/configuration/resolve/#resolvealias) | "Utilities" |
| aliased-workspace | the module was imported via a [workspace](https://docs.npmjs.com/cli/v10/configuring-npm/package-json#workspaces) | "local-workspace-package" |
| unknown | it's unknown what kind of dependency type this is - probably because the module could not be resolved in the first place | "loodash" |
| undetermined | the dependency fell through all detection holes. This could happen with amd dependencies - which have a whole Jurassic park of ways to define where to resolve modules to | "veloci!./raptor" |
| type-only | the module was imported as 'type only' (e.g. `import type { IThing } from "./things";`) - only available for TypeScript sources, and only when tsPreCompilationDeps !== false | |
Expand Down
94 changes: 86 additions & 8 deletions src/extract/resolve/module-classifiers.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { isAbsolute, resolve as path_resolve } from "node:path";
import { isMatch } from "picomatch";
import getExtension from "#utl/get-extension.mjs";

let gFollowableExtensionsCache = new Set();
Expand Down Expand Up @@ -89,14 +90,7 @@ export function isFollowable(pResolvedFilename, pResolveOptions) {
);
}

function isWebPackAliased(pModuleName, pAliasObject) {
return Object.keys(pAliasObject || {}).some((pAliasLHS) =>
pModuleName.startsWith(pAliasLHS),
);
}

/**
*
* @param {string} pModuleName
* @param {object} pManifest
* @returns {boolean}
Expand All @@ -105,6 +99,11 @@ function isSubpathImport(pModuleName, pManifest) {
return (
pModuleName.startsWith("#") &&
Object.keys(pManifest?.imports ?? {}).some((pImportLHS) => {
// Although they might look as such, the LHS of an import statement
// (a 'subpath pattern') is not a glob. The * functions as string
// replacement only.
// Quoting https://nodejs.org/api/packages.html#subpath-imports :
// > "* maps expose nested subpaths as it is a string replacement syntax only"
const lMatchREasString = pImportLHS.replace(/\*/g, ".+");
// eslint-disable-next-line security/detect-non-literal-regexp
const lMatchRE = new RegExp(`^${lMatchREasString}$`);
Expand All @@ -113,6 +112,83 @@ function isSubpathImport(pModuleName, pManifest) {
);
}

/**
* @param {string} pModuleName
* @param {object} pAliasObject
* @returns {boolean}
*/
function isWebPackAliased(pModuleName, pAliasObject) {
return Object.keys(pAliasObject || {}).some((pAliasLHS) =>
pModuleName.startsWith(pAliasLHS),
);
}

/**
* @param {string} pModuleName
* @param {string} pResolvedModuleName
* @param {object} pManifest
* @returns {boolean}
*/
// 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) {
// workspaces are an array of globs that match the (sub) workspace
// folder itself only.
//
// workspaces: [
// "packages/*", -> matches packages/a, packages/b, packages/c, ...
// "libs/x", -> matches libs/x
// "libs/y", -> matches libs/y
// "apps" -> matches apps
// ]
//
// By definition this will _never_ match resolved module names.
// E.g. in packages/a => packages/a/dist/main/index.js or
// in libs/x => libs/x/index.js
//
// This is why we chuck a `/**` at the end of each workspace glob, which
// transforms it into a 'starts with' glob. And yeah, you can have a /
// at the end of a glob. And because double slashes are taken literally
// we have a ternary operator to prevent those.
//
// 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}/**`,
);
if (isMatch(pResolvedModuleName, lModuleFriendlyWorkspaceGlobs)) {
return true;
}
// it's possible to run node with --preserve-symlinks, in which case
// the symlinked workspace folders are not resolved to their realpath.
// So we need to check both the thing in node_modules _and_ the resolved
// thing. Annoyingly, the symlink in node_modules is the `name` attribute
// of the workspace, not the path of the workspace itself. So if it's
// in node_modules we need to check against the unresolved modulename.
//
// Other then the detection for when symlinks are resolved to their realpath
// (the if above), this is a 'best effort' detection only for now; there's
// guaranteed to be scenarios where this will fail. How often is the
// --preserve-symlinks flag used in practice, though?
const lModuleFriendlyWorkspaceGlobsWithNodeModules =
lModuleFriendlyWorkspaceGlobs.map(
(pWorkspace) => `(node_modules/)?${pWorkspace}`,
);
return isMatch(pModuleName, lModuleFriendlyWorkspaceGlobsWithNodeModules);
}
return false;
}

/**
* @param {string} pModuleName
* @param {string} pResolvedModuleName
* @param {import("../../../types/resolve-options").IResolveOptions} pResolveOptions
* @param {string} pBaseDirectory
* @returns {boolean}
*/
function isLikelyTSAliased(
pModuleName,
pResolvedModuleName,
Expand All @@ -128,7 +204,6 @@ function isLikelyTSAliased(
}

/**
*
* @param {string} pModuleName
* @param {string} pResolvedModuleName
* @param {import("../../../types/resolve-options").IResolveOptions} pResolveOptions
Expand All @@ -147,6 +222,9 @@ export function getAliasTypes(
if (isWebPackAliased(pModuleName, pResolveOptions.alias)) {
return ["aliased", "aliased-webpack"];
}
if (isWorkspaceAliased(pModuleName, pResolvedModuleName, pManifest)) {
return ["aliased", "aliased-workspace"];
}
if (
isLikelyTSAliased(
pModuleName,
Expand Down
3 changes: 2 additions & 1 deletion src/schema/configuration.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit fce008d

Please sign in to comment.