Skip to content

Commit

Permalink
feat!(extract): makes 'real' depencency type available alongside alia…
Browse files Browse the repository at this point in the history 
…ses BREAKING (#856)

## Description, Motivation and Context

Before this PR dependency types that came out of an alias (subpath
import, webpack alias, tsconfig path) only stated that the dependency
was an alias. However, these aliases point to other dependency types -
local modules, third party ('npm') modules, nodejs builtin core(!)
modules. This is useful information, and actually not listing those is
an omission.

After merging this PR dependency-cruiser will start listing the 'actual'
dependency type _alongside_ the alias dependency types.

Technically this is a BREAKING CHANGE because rules that triggered on
these 'real' dependency types before didn't trigger because they weren't
detected.


### after

```javascript
{
  "modules": [
    {
      "source": "bin/depcruise-baseline.mjs",
      "dependencies": [
        {
          "dynamic": false,
          "module": "#cli/assert-node-environment-suitable.mjs",
          "moduleSystem": "es6",
          "exoticallyRequired": false,
          "resolved": "src/cli/assert-node-environment-suitable.mjs",
          "coreModule": false,
          "followable": true,
          "couldNotResolve": false,
          "dependencyTypes": [
            "aliased",
            "aliased-subpath-import",
            "local" // <<---- Now also listed that it's a local dependency
          ],
          "matchesDoNotFollow": false,
          "circular": false,
          "valid": true
        },
// ...
```

> If you have a rule that e.g. forbids the `local` dependencyType from
being used
> it before didn't trigger on the above dependency. It now will, however
...

### before
```javascript
{
  "modules": [
    {
      "source": "bin/depcruise-baseline.mjs",
      "dependencies": [
        {
          "dynamic": false,
          "module": "#cli/assert-node-environment-suitable.mjs",
          "moduleSystem": "es6",
          "exoticallyRequired": false,
          "resolved": "src/cli/assert-node-environment-suitable.mjs",
          "coreModule": false,
          "followable": true,
          "couldNotResolve": false,
          "dependencyTypes": [
            "aliased",
            "aliased-subpath-import"
          ],
          "matchesDoNotFollow": false,
          "circular": false,
          "valid": true
        },
// ...
```

## How Has This Been Tested?

- [x] green ci
- [x] additional & adapted automated non-regression tests

## 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)
- [x] Breaking change (fix or feature that would cause existing
functionality to change)
  • Loading branch information
@sverweij
sverweij committed Oct 22, 2023
1 parent c9288e3 commit a3cfcec
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 65 deletions.
40 changes: 20 additions & 20 deletions doc/rules-reference.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -918,26 +918,26 @@ will ignore them in the evaluation of that rule.

This is a list of dependency types dependency-cruiser currently detects.

| dependency type | meaning | example |
| ---------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------- |
| local | a module in your own ('local') package | "./klont" |
| localmodule | a module in your own ('local') package, but which was in the `resolve.modules` attribute of the webpack config you passed | "shared/stuff.ts" |
| npm | it's a module in package.json's `dependencies` | "lodash" |
| npm-dev | it's a module in package.json's `devDependencies` | "chai" |
| npm-optional | it's a module in package.json's `optionalDependencies` | "livescript" |
| npm-peer | it's a module in package.json's `peerDependencies` - note: deprecated in npm 3 | "thing-i-am-a-plugin-for" |
| npm-bundled | it's a module that occurs in package.json's `bundle(d)Dependencies` array | "iwillgetbundled" |
| npm-no-pkg | it's an npm module - but it's nowhere in your package.json | "forgetmenot" |
| npm-unknown | it's an npm module - but there is no (parseable/ valid) package.json in your package | |
| deprecated | it's an npm module, but the version you're using or the module itself is officially deprecated | "some-deprecated-package" |
| core | it's a core module | "fs" |
| aliased | the module was imported via an alias - always occurs alongside one of 'aliased-subpath-import', 'aliased-webpack' and 'aliased-tsconfig' dependency types | "~/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" |
| 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 | |
| dependency type | meaning | example |
| ---------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------- |
| local | a module in your own ('local') package | "./klont" |
| localmodule | a module in your own ('local') package, but which was in the `resolve.modules` attribute of the webpack config you passed | "shared/stuff.ts" |
| npm | it's a module in package.json's `dependencies` | "lodash" |
| npm-dev | it's a module in package.json's `devDependencies` | "chai" |
| npm-optional | it's a module in package.json's `optionalDependencies` | "livescript" |
| npm-peer | it's a module in package.json's `peerDependencies` - note: deprecated in npm 3 | "thing-i-am-a-plugin-for" |
| npm-bundled | it's a module that occurs in package.json's `bundle(d)Dependencies` array | "iwillgetbundled" |
| npm-no-pkg | it's an npm module - but it's nowhere in your package.json | "forgetmenot" |
| npm-unknown | it's an npm module - but there is no (parseable/ valid) package.json in your package | |
| deprecated | it's an npm module, but the version you're using or the module itself is officially deprecated | "some-deprecated-package" |
| 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" |
| 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 | |

### `dynamic`

Expand Down
60 changes: 37 additions & 23 deletions src/extract/resolve/determine-dependency-types.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ function determineExternalModuleDependencyTypes(
* @param {string} pModuleName the module name as found in the source
* @param {any} pManifest a package.json, in object format
* @param {string} pFileDirectory the directory relative to which to resolve (only used for npm deps here)
* @param {any} pResolveOptions an enhanced resolve 'resolve' key
* @param {import("../../../types/resolve-options.js").IResolveOptions} pResolveOptions an enhanced resolve 'resolve' key
* @param {string} pBaseDirectory the base directory dependency cruise is run on
*
* @return {import("../../../types/shared-types.js").DependencyType[]} an array of dependency types for the dependency
Expand All @@ -184,45 +184,59 @@ export default function determineDependencyTypes(
pBaseDirectory,
) {
/** @type {import("../../../types/shared-types.js").DependencyType[]}*/
let lReturnValue = ["undetermined"];
let lReturnValue = [];
const lResolveOptions = pResolveOptions || {};

if (pDependency.couldNotResolve) {
lReturnValue = ["unknown"];
} else if (pDependency.coreModule) {
return ["unknown"];
}

const lAliases = getAliasTypes(
pModuleName,
pDependency.resolved,
lResolveOptions,
pManifest,
);
if (lAliases.length > 0) {
lReturnValue = lAliases;
}

if (pDependency.coreModule) {
// this business seems duplicate (it's already in
// the passed object as `coreModule`- determined by the resolve-AMD or
// resolve-commonJS module). I want to deprecate the `coreModule`
// attribute in favor of this one and determining it here will make
// live easier in the future
lReturnValue = ["core"];
} else if (isRelativeModuleName(pModuleName)) {
lReturnValue = ["local"];
lReturnValue.push("core");
} else if (
isRelativeModuleName(pModuleName) ||
(lAliases.length > 0 &&
!isExternalModule(
pDependency.resolved,
lResolveOptions.modules,
pBaseDirectory,
))
) {
lReturnValue.push("local");
} else if (
isExternalModule(
pDependency.resolved,
lResolveOptions.modules,
pBaseDirectory,
)
) {
lReturnValue = determineExternalModuleDependencyTypes(
pDependency,
pModuleName,
pManifest,
pFileDirectory,
lResolveOptions,
pBaseDirectory,
lReturnValue = lReturnValue.concat(
determineExternalModuleDependencyTypes(
pDependency,
pModuleName,
pManifest,
pFileDirectory,
lResolveOptions,
pBaseDirectory,
),
);
} else {
const lAliases = getAliasTypes(
pModuleName,
pDependency.resolved,
lResolveOptions,
pManifest,
);
if (lAliases.length > 0) {
lReturnValue = lAliases;
}
lReturnValue.push("undetermined");
}

return lReturnValue.concat(pDependency.dependencyTypes || []);
Expand Down
2 changes: 1 addition & 1 deletion src/extract/resolve/module-classifiers.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ function isWebPackAliased(pModuleName, pAliasObject) {
function isSubpathImport(pModuleName, pManifest) {
return (
pModuleName.startsWith("#") &&
Object.keys(pManifest?.imports || {}).some((pImportLHS) => {
Object.keys(pManifest?.imports ?? {}).some((pImportLHS) => {
const lMatchREasString = pImportLHS.replace(/\*/g, ".+");
// eslint-disable-next-line security/detect-non-literal-regexp
const lMatchRE = new RegExp(`^${lMatchREasString}$`);
Expand Down
4 changes: 2 additions & 2 deletions test/cli/__fixtures__/typescript-path-resolution.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"coreModule": false,
"followable": true,
"couldNotResolve": false,
"dependencyTypes": ["aliased", "aliased-tsconfig"],
"dependencyTypes": ["aliased", "aliased-tsconfig", "local"],
"matchesDoNotFollow": false,
"circular": false,
"valid": true
Expand Down Expand Up @@ -44,7 +44,7 @@
"coreModule": false,
"followable": true,
"couldNotResolve": false,
"dependencyTypes": ["aliased", "aliased-tsconfig"],
"dependencyTypes": ["aliased", "aliased-tsconfig", "local"],
"matchesDoNotFollow": false,
"circular": false,
"valid": true
Expand Down
6 changes: 3 additions & 3 deletions test/cli/__fixtures__/webpack-config-alias-cruiser-config.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"coreModule": false,
"followable": true,
"couldNotResolve": false,
"dependencyTypes": ["aliased", "aliased-webpack"],
"dependencyTypes": ["aliased", "aliased-webpack", "local"],
"matchesDoNotFollow": false,
"circular": false,
"valid": true
Expand All @@ -35,7 +35,7 @@
"coreModule": false,
"followable": false,
"couldNotResolve": false,
"dependencyTypes": ["aliased", "aliased-webpack"],
"dependencyTypes": ["aliased", "aliased-webpack", "local"],
"matchesDoNotFollow": false,
"circular": false,
"valid": true
Expand All @@ -51,7 +51,7 @@
"coreModule": false,
"couldNotResolve": false,
"matchesDoNotFollow": false,
"dependencyTypes": ["aliased", "aliased-webpack"],
"dependencyTypes": ["aliased", "aliased-webpack", "local"],
"dependencies": [],
"dependents": [
"test/cli/__fixtures__/webpackconfig/aliassy/src/index.js"
Expand Down
6 changes: 3 additions & 3 deletions test/cli/__fixtures__/webpack-config-alias.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"coreModule": false,
"followable": true,
"couldNotResolve": false,
"dependencyTypes": ["aliased", "aliased-webpack"],
"dependencyTypes": ["aliased", "aliased-webpack", "local"],
"matchesDoNotFollow": false,
"circular": false,
"valid": true
Expand All @@ -35,7 +35,7 @@
"coreModule": false,
"followable": false,
"couldNotResolve": false,
"dependencyTypes": ["aliased", "aliased-webpack"],
"dependencyTypes": ["aliased", "aliased-webpack", "local"],
"matchesDoNotFollow": false,
"circular": false,
"valid": true
Expand All @@ -51,7 +51,7 @@
"coreModule": false,
"couldNotResolve": false,
"matchesDoNotFollow": false,
"dependencyTypes": ["aliased", "aliased-webpack"],
"dependencyTypes": ["aliased", "aliased-webpack", "local"],
"dependencies": [],
"dependents": [
"test/cli/__fixtures__/webpackconfig/aliassy/src/index.js"
Expand Down
57 changes: 51 additions & 6 deletions test/extract/resolve/determine-dependency-types.spec.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -243,15 +243,15 @@ describe("[U] extract/resolve/determineDependencyTypes - determine dependencyTyp
"#the-alias",
{
imports: {
"#the-alias": "src/i-was-aliased.js",
"#the-alias": "./src/i-was-aliased.js",
},
},
".",
{
modules: ["node_modules"],
},
),
["aliased", "aliased-subpath-import"],
["aliased", "aliased-subpath-import", "local"],
);
});

Expand All @@ -265,15 +265,60 @@ describe("[U] extract/resolve/determineDependencyTypes - determine dependencyTyp
"#some/folder/source.mjs",
{
imports: {
"#*": "src/*",
"#*": "./src/*",
},
},
".",
{
modules: ["node_modules"],
},
),
["aliased", "aliased-subpath-import", "local"],
);
});

it("classifies subpath imports that resolve to an external modules both as external module and as aliased", () => {
deepEqual(
determineDependencyTypes(
{
couldNotResolve: false,
resolved: "node_modules/cool-module/main/index.js",
},
"#the-alias",
{
imports: {
"#the-alias": "cool-module",
},
},
".",
{
modules: ["node_modules"],
},
),
["aliased", "aliased-subpath-import", "npm-no-pkg"],
);
});

it("classifies subpath imports that resolve to an builtin module both as core module and as aliased", () => {
deepEqual(
determineDependencyTypes(
{
couldNotResolve: false,
resolved: "test",
coreModule: true,
},
"#the-alias",
{
imports: {
"#the-alias": "test",
},
},
".",
{
modules: ["node_modules"],
},
),
["aliased", "aliased-subpath-import"],
["aliased", "aliased-subpath-import", "core"],
);
});

Expand Down Expand Up @@ -321,7 +366,7 @@ describe("[U] extract/resolve/determineDependencyTypes - determine dependencyTyp
},
// pBaseDirectory
),
["aliased", "aliased-webpack"],
["aliased", "aliased-webpack", "local"],
);
});

Expand All @@ -345,7 +390,7 @@ describe("[U] extract/resolve/determineDependencyTypes - determine dependencyTyp
},
// pBaseDirectory
),
["aliased", "aliased-tsconfig"],
["aliased", "aliased-tsconfig", "local"],
);
});

Expand Down
2 changes: 1 addition & 1 deletion test/extract/resolve/index.general.spec.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ describe("[I] extract/resolve/index - general", () => {
{
coreModule: false,
couldNotResolve: false,
dependencyTypes: ["aliased", "aliased-webpack"],
dependencyTypes: ["aliased", "aliased-webpack", "local"],
followable: true,
resolved: "i-got-aliased-to-hoepla/hoi/index.js",
},
Expand Down

0 comments on commit a3cfcec

Please sign in to comment.