Skip to content

Commit

Permalink
fix(resolve)!: makes alias type derivation more correct and precise B…
Browse files Browse the repository at this point in the history 
…REAKING (#867)

## Description

- takes 'workspaces' field into account when merging package manifests
- same for the 'imports' field
- when determining what kind of an alias a thing is use the _parsed_
tsconfig to determine it
- adds `alias-tsconfig-base-url` and `alias-tsconfig-paths` dependency
types to distinguish between the two types of aliases one can use within
tsconfigs. B.t.w. for both dependency-cruiser will keep emitting the
`alias` and `alias-tsconfig` dependency types as well - both for
convenience and backwards compatibility.


This PR is theoretically 🚨 BREAKING as rules previously defined on
aliases, that didn't fire now _will_ because they're now detected
correctly.

## Motivation and Context

Now we're doing explicit things with `workspaces` and `imports` fields
we need to have them in merged package manifests as well, so in
monorepos we still take them into account. The rewrite of the tsconfig
alias detection logic is necessary to ensure we detect tsconfig aliases
correctly and more precisely.


Also see #863.

## How Has This Been Tested?

- [x] green ci
- [x] additional unit tests
- [x] additional integration test(s)

## Types of changes

- [x] 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 Dec 9, 2023
1 parent 83fafe1 commit 03bdda2
Show file tree
Hide file tree
Showing 47 changed files with 853 additions and 172 deletions.
44 changes: 23 additions & 21 deletions doc/rules-reference.md
View file
Expand Up @@ -918,27 +918,29 @@ 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-\*' 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-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 | |
| 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, but later on revived. | "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 (nodejs) core module. B.t.w. you can [influence](./options-reference.md#builtinmodules-influencing-what-to-consider-built-in--core-modules) what to consider a core module if | "fs", "node:test" |
| 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-tsconfig | the module was imported via a typescript compilerOptions.paths or compilerOptions.baseUrl setting in tsconfig. Always occurs alongside one of the 'aliased-tsconfig-\*' types below | "@thing/hello" |
| aliased-tsconfig-base-url | the module was imported via a typescript [compilerOptions.baseUrl setting in tsconfig](https://www.typescriptlang.org/tsconfig#baseUrl) | "libs/utensils/src/hello.js" |
| aliased-tsconfig-paths | 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, only for tsPreCompilationDeps !== false. Will appear alongside other types. | |

### `dynamic`

Expand Down
2 changes: 1 addition & 1 deletion src/cli/index.mjs
View file
Expand Up @@ -137,7 +137,7 @@ async function runCruise(pFileDirectoryArray, pCruiseOptions) {
/**
*
* @param {string[]} pFileDirectoryArray
* @param {import("../../types/options").ICruiseOptions} lCruiseOptions
* @param {import("../../types/options.mjs").ICruiseOptions} lCruiseOptions
* @returns {number}
*/
export default async function executeCli(pFileDirectoryArray, pCruiseOptions) {
Expand Down
11 changes: 10 additions & 1 deletion src/extract/get-dependencies.mjs
View file
Expand Up @@ -160,13 +160,15 @@ function addResolutionAttributes(
{ baseDir, doNotFollow },
pFileName,
pResolveOptions,
pTranspileOptions,
) {
return function addAttributes(pDependency) {
const lResolved = resolve(
pDependency,
baseDir,
join(baseDir, dirname(pFileName)),
pResolveOptions,
pTranspileOptions,
);
const lMatchesDoNotFollow = matchesDoNotFollow(lResolved, doNotFollow);

Expand Down Expand Up @@ -231,7 +233,14 @@ export default function getDependencies(
getDependencyUniqueKey,
)
.sort(compareDeps)
.map(addResolutionAttributes(pCruiseOptions, pFileName, pResolveOptions))
.map(
addResolutionAttributes(
pCruiseOptions,
pFileName,
pResolveOptions,
pTranspileOptions,
),
)
.filter(
({ resolved }) =>
(!pCruiseOptions?.exclude?.path ||
Expand Down
3 changes: 3 additions & 0 deletions src/extract/resolve/determine-dependency-types.mjs
View file
Expand Up @@ -171,6 +171,7 @@ function determineExternalModuleDependencyTypes(
* @param {string} pFileDirectory the directory relative to which to resolve (only used for npm deps here)
* @param {import("../../../types/resolve-options.mjs").IResolveOptions} pResolveOptions an enhanced resolve 'resolve' key
* @param {string} pBaseDirectory the base directory dependency cruise is run on
* @param {import("../../../types/dependency-cruiser.mjs").ITranspileOptions} pTranspileOptions
*
* @return {import("../../../types/shared-types.js").DependencyType[]} an array of dependency types for the dependency
*/
Expand All @@ -182,6 +183,7 @@ export default function determineDependencyTypes(
pFileDirectory,
pResolveOptions,
pBaseDirectory,
pTranspileOptions,
) {
/** @type {import("../../../types/shared-types.js").DependencyType[]}*/
let lReturnValue = [];
Expand All @@ -196,6 +198,7 @@ export default function determineDependencyTypes(
pDependency.resolved,
lResolveOptions,
pManifest,
pTranspileOptions,
);
if (lAliases.length > 0) {
lReturnValue = lAliases;
Expand Down
6 changes: 5 additions & 1 deletion src/extract/resolve/index.mjs
View file
Expand Up @@ -160,15 +160,18 @@ function resolveWithRetry(
* @param {string} pFileDirectory the directory of the file the dependency was
* detected in
* @param {import("../../../types/resolve-options.mjs").IResolveOptions} pResolveOptions
* @param {any} pTranspileOptions
* @return {Partial <import("../../../types/cruise-result.mjs").IDependency>}
*
*
*/
// eslint-disable-next-line max-lines-per-function
// eslint-disable-next-line max-lines-per-function, max-params
export default function resolve(
pDependency,
pBaseDirectory,
pFileDirectory,
pResolveOptions,
pTranspileOptions,
) {
let lResolvedDependency = resolveWithRetry(
pDependency,
Expand Down Expand Up @@ -197,6 +200,7 @@ export default function resolve(
pFileDirectory,
pResolveOptions,
pBaseDirectory,
pTranspileOptions,
),
};

Expand Down
14 changes: 10 additions & 4 deletions src/extract/resolve/merge-manifests.mjs
View file
Expand Up @@ -24,12 +24,14 @@ function mergeDependencyArray(pClosestDependencyKey, pFurtherDependencyKey) {
return uniq(pClosestDependencyKey.concat(pFurtherDependencyKey));
}

function isDependencyKey(pKey) {
return pKey.endsWith("ependencies");
function isInterestingKey(pKey) {
return (
pKey.endsWith("ependencies") || pKey === "workspaces" || pKey === "imports"
);
}

function getDependencyKeys(pPackage) {
return Object.keys(pPackage).filter(isDependencyKey);
return Object.keys(pPackage).filter(isInterestingKey);
}

function getJointUniqueDependencyKeys(pClosestPackage, pFurtherPackage) {
Expand All @@ -40,6 +42,10 @@ function getJointUniqueDependencyKeys(pClosestPackage, pFurtherPackage) {
);
}

function isAnArrayKey(pKey) {
return pKey.startsWith("bundle") || pKey === "workspaces";
}

/**
* returns an object with
* - the *dependencies keys from both passed packages
Expand All @@ -63,7 +69,7 @@ export default function mergeManifests(pClosestManifest, pFurtherManifest) {
)
.map((pKey) => ({
key: pKey,
value: pKey.startsWith("bundle")
value: isAnArrayKey(pKey)
? mergeDependencyArray(
pClosestManifest?.[pKey] ?? [],
pFurtherManifest?.[pKey] ?? [],
Expand Down

0 comments on commit 03bdda2

Please sign in to comment.