Skip to content

Commit

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

- ensures that subpath imports are recognized as 'aliased', just like
the webpack and tsconfig aliases & import paths.
- splits out the 'aliased' dependency type into three (while leaving the
'aliased' one in place both for convenience and backwards
compatibility): `aliased-subpath-import`, `aliased-webpack`,
`aliased-tsconfig`
- modifies the `moreThanOneDependencyType` rule/ matcher so ...
- it doesn't count aliases as separate dependency types (see
documentation for rationale)
- it doesn't count `type-only` as separate dependency types (also: see
documentation)

TODO: subpath imports can resolve to local modules (`"#*": "./src/*"`),
_but also_ to 3rd party modules (`"#aliasdash/*": "lodash/*"`) or even
core modules (which should be considered a criminal offence and is
currently not even recognized in enhanced-resolve: `"#path-but-aliased":
"path"`). These should be added to the dependencyTypes array as well -
will probably be in a separate PR, though.

## Motivation and Context

[subpath imports](https://nodejs.org/api/packages.html#subpath-imports)
work since node 12.9 (!) and are a vastly superior alternative to
tsconfig paths other alias systems (in webpack, babel, ...):
- there's native support for it in nodejs, typescript (recent versions)
and most other bundler/ packager under the sun.
- This means it doesn't require third party shims/ polyfills, which is
good for site reliability.

Dependency-cruiser already correctly resolved subpath imports - this PR
ensures they're explicitly named as such.


## How Has This Been Tested?

- [x] green ci
- [x] additional automated non-regression tests
- [x] updated 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)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
  • Loading branch information
@sverweij
sverweij committed Oct 21, 2023
1 parent 08dce74 commit 36c3dde
Show file tree
Hide file tree
Showing 20 changed files with 309 additions and 60 deletions.
42 changes: 25 additions & 17 deletions doc/rules-reference.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -918,23 +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 | it's a module that's linked through an aliased (webpack) | "~/hello.ts" |
| 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-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 | |

### `dynamic`

Expand Down Expand Up @@ -992,6 +995,11 @@ when set to true:
}
```

For this rule the `aliased`, `aliased-subpath-import`, `aliased-webpack`,
`aliased-tsconfig` and `type-only` dependency types do not count towards the
number of dependency types a dependency has, as these _by definition_ are
always in addition to one of the other dependency types.

When left out it doesn't matter how many dependency types a dependency has.

(If you're more of an 'allowed' user: it matches the 0 and 1 cases when set to
Expand Down
43 changes: 26 additions & 17 deletions src/extract/resolve/determine-dependency-types.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import has from "lodash/has.js";
import {
isRelativeModuleName,
isExternalModule,
isAliassy,
getAliasTypes,
} from "./module-classifiers.mjs";
import {
dependencyIsDeprecated,
Expand All @@ -13,7 +13,7 @@ import {
function dependencyKeyHasModuleName(
pPackageDependencies,
pModuleName,
pPrefix
pPrefix,
) {
return (pKey) =>
pKey.includes("ependencies") &&
Expand All @@ -30,25 +30,25 @@ const NPM2DEP_TYPE = new Map([
function findModuleInPackageDependencies(
pPackageDependencies,
pModuleName,
pPrefix
pPrefix,
) {
return Object.keys(pPackageDependencies)
.filter(
dependencyKeyHasModuleName(pPackageDependencies, pModuleName, pPrefix)
dependencyKeyHasModuleName(pPackageDependencies, pModuleName, pPrefix),
)
.map((pKey) => NPM2DEP_TYPE.get(pKey) || "npm-no-pkg");
}

function needToLookAtTypesToo(pResolverModulePaths) {
return (pResolverModulePaths || ["node_modules", "node_modules/@types"]).some(
(pPath) => pPath.includes("@types")
(pPath) => pPath.includes("@types"),
);
}

function determineManifestDependencyTypes(
pModuleName,
pPackageDependencies,
pResolverModulePaths
pResolverModulePaths,
) {
/** @type {import("../../../types/shared-types.js").DependencyType[]} */
let lReturnValue = ["npm-unknown"];
Expand All @@ -57,7 +57,7 @@ function determineManifestDependencyTypes(
lReturnValue = findModuleInPackageDependencies(
pPackageDependencies,
pModuleName,
""
"",
);

if (
Expand All @@ -67,7 +67,7 @@ function determineManifestDependencyTypes(
lReturnValue = findModuleInPackageDependencies(
pPackageDependencies,
pModuleName,
"@types"
"@types",
);
}
lReturnValue = lReturnValue.length === 0 ? ["npm-no-pkg"] : lReturnValue;
Expand Down Expand Up @@ -106,13 +106,13 @@ function determineNodeModuleDependencyTypes(
pModuleName,
pPackageDeps,
pFileDirectory,
pResolveOptions
pResolveOptions,
) {
/** @type {import("../../../types/shared-types.js").DependencyType[]} */
let lReturnValue = determineManifestDependencyTypes(
getPackageRoot(pModuleName),
pPackageDeps,
pResolveOptions.modules
pResolveOptions.modules,
);
if (
pResolveOptions.resolveDeprecations &&
Expand Down Expand Up @@ -142,7 +142,7 @@ function determineExternalModuleDependencyTypes(
pPackageDeps,
pFileDirectory,
pResolveOptions,
pBaseDirectory
pBaseDirectory,
) {
/** @type {import("../../../types/shared-types.js").DependencyType[]} */
let lReturnValue = [];
Expand All @@ -154,7 +154,7 @@ function determineExternalModuleDependencyTypes(
pModuleName,
pPackageDeps,
pFileDirectory,
pResolveOptions
pResolveOptions,
);
} else {
lReturnValue = ["localmodule"];
Expand All @@ -174,13 +174,14 @@ function determineExternalModuleDependencyTypes(
*
* @return {import("../../../types/shared-types.js").DependencyType[]} an array of dependency types for the dependency
*/
// eslint-disable-next-line max-lines-per-function
export default function determineDependencyTypes(
pDependency,
pModuleName,
pManifest,
pFileDirectory,
pResolveOptions,
pBaseDirectory
pBaseDirectory,
) {
/** @type {import("../../../types/shared-types.js").DependencyType[]}*/
let lReturnValue = ["undetermined"];
Expand All @@ -201,7 +202,7 @@ export default function determineDependencyTypes(
isExternalModule(
pDependency.resolved,
lResolveOptions.modules,
pBaseDirectory
pBaseDirectory,
)
) {
lReturnValue = determineExternalModuleDependencyTypes(
Expand All @@ -210,10 +211,18 @@ export default function determineDependencyTypes(
pManifest,
pFileDirectory,
lResolveOptions,
pBaseDirectory
pBaseDirectory,
);
} else {
const lAliases = getAliasTypes(
pModuleName,
pDependency.resolved,
lResolveOptions,
pManifest,
);
} else if (isAliassy(pModuleName, pDependency.resolved, lResolveOptions)) {
lReturnValue = ["aliased"];
if (lAliases.length > 0) {
lReturnValue = lAliases;
}
}

return lReturnValue.concat(pDependency.dependencyTypes || []);
Expand Down
54 changes: 49 additions & 5 deletions src/extract/resolve/module-classifiers.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,24 @@ function isWebPackAliased(pModuleName, pAliasObject) {
);
}

/**
*
* @param {string} pModuleName
* @param {object} pManifest
* @returns {boolean}
*/
function isSubpathImport(pModuleName, pManifest) {
return (
pModuleName.startsWith("#") &&
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}$`);
return Boolean(pModuleName.match(lMatchRE));
})
);
}

function isLikelyTSAliased(
pModuleName,
pResolvedModuleName,
Expand All @@ -109,9 +127,35 @@ function isLikelyTSAliased(
);
}

export function isAliassy(pModuleName, pResolvedModuleName, pResolveOptions) {
return (
isWebPackAliased(pModuleName, pResolveOptions.alias) ||
isLikelyTSAliased(pModuleName, pResolvedModuleName, pResolveOptions)
);
/**
*
* @param {string} pModuleName
* @param {string} pResolvedModuleName
* @param {import("../../../types/resolve-options").IResolveOptions} pResolveOptions
* @param {object} pManifest
* @returns {string[]}
*/
export function getAliasTypes(
pModuleName,
pResolvedModuleName,
pResolveOptions,
pManifest,
) {
if (isSubpathImport(pModuleName, pManifest)) {
return ["aliased", "aliased-subpath-import"];
}
if (isWebPackAliased(pModuleName, pResolveOptions.alias)) {
return ["aliased", "aliased-webpack"];
}
if (
isLikelyTSAliased(
pModuleName,
pResolvedModuleName,
pResolveOptions,
pResolveOptions.baseDirectory,
)
) {
return ["aliased", "aliased-tsconfig"];
}
return [];
}
3 changes: 3 additions & 0 deletions src/schema/configuration.schema.json
View file

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

0 comments on commit 36c3dde

Please sign in to comment.