Skip to content

Commit

Permalink
fix(extract): also detects node:test as a core module (#822)
Browse files Browse the repository at this point in the history
## Description

- ensures `node:test` is detected as a core module

## Motivation and Context

Currently `node:test` dependencies are classified as unable to resolve.
This is because `node:test` or `test` are not listed as `builtinModules`
in node's `module` module. From the discussion on
nodejs/node#42785 I understand this is never
going to happen either. Alternative is to use `module`'s `isBuiltin()`
function, but that is not available in the lowest version of nodejs
dependency-cruiser supports (16.14), so we have to work around that.

Additionally the `node:test` module can _only_ be required via the
`node:` protocol (`const { test} = require('test')`/ `import { test }
from 'test'` don't work). This means that with the current the way we
split the protocol from the module name, using `isBuiltin` won't work
either...

## How Has This Been Tested?

- [x] green ci
- [x] additional automated tests

## 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)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)

## Checklist

- [x] 📖

  - My change doesn't require a documentation update, or ...
  - it _does_ and I have updated it

- [x] ⚖️
- The contribution will be subject to [The MIT
license](https://github.com/sverweij/dependency-cruiser/blob/main/LICENSE),
and I'm OK with that.
  - The contribution is my own original work.
- I am ok with the stuff in
[**CONTRIBUTING.md**](https://github.com/sverweij/dependency-cruiser/blob/main/.github/CONTRIBUTING.md).
  • Loading branch information
sverweij authored Jul 15, 2023
1 parent 10827d7 commit 231a399
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 14 deletions.
22 changes: 15 additions & 7 deletions src/extract/resolve/resolve-amd.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@ import { builtinModules } from "node:module";
import memoize from "lodash/memoize.js";
import pathToPosix from "../../utl/path-to-posix.mjs";

// builtinModules does not expose all builtin modules for #reasons -
// see https://github.com/nodejs/node/issues/42785. In stead we could use
// isBuiltin, but that is not available in node 16.14, the lowest version
// of node dependency-cruiser currently supports. So we add the missing
// modules here.
// b.t.w. code is duplicated in resolve-cjs.mjs
const REALLY_BUILTIN_MODULES = builtinModules.concat(["test", "node:test"]);

const fileExists = memoize((pFile) => {
try {
accessSync(pFile, R_OK);
Expand All @@ -16,7 +24,7 @@ const fileExists = memoize((pFile) => {

function guessPath(pBaseDirectory, pFileDirectory, pStrippedModuleName) {
return pathToPosix(
relative(pBaseDirectory, join(pFileDirectory, pStrippedModuleName))
relative(pBaseDirectory, join(pFileDirectory, pStrippedModuleName)),
);
}

Expand All @@ -27,8 +35,8 @@ function guessLikelyPath(pBaseDirectory, pFileDirectory, pStrippedModuleName) {
guessPath(
pBaseDirectory,
pFileDirectory,
`${pStrippedModuleName}${pExtension}`
)
`${pStrippedModuleName}${pExtension}`,
),
)
.find(fileExists) || pStrippedModuleName
);
Expand All @@ -37,7 +45,7 @@ function guessLikelyPath(pBaseDirectory, pFileDirectory, pStrippedModuleName) {
export function resolveAMD(
pStrippedModuleName,
pBaseDirectory,
pFileDirectory
pFileDirectory,
) {
// lookups:
// - [x] could be relative in the end (implemented)
Expand All @@ -48,15 +56,15 @@ export function resolveAMD(
const lResolvedPath = guessLikelyPath(
pBaseDirectory,
pFileDirectory,
pStrippedModuleName
pStrippedModuleName,
);

return {
resolved: lResolvedPath,
coreModule: builtinModules.includes(pStrippedModuleName),
coreModule: REALLY_BUILTIN_MODULES.includes(pStrippedModuleName),
followable: fileExists(lResolvedPath) && lResolvedPath.endsWith(".js"),
couldNotResolve:
!builtinModules.includes(pStrippedModuleName) &&
!REALLY_BUILTIN_MODULES.includes(pStrippedModuleName) &&
!fileExists(lResolvedPath),
};
}
Expand Down
22 changes: 15 additions & 7 deletions src/extract/resolve/resolve-cjs.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,35 @@ import pathToPosix from "../../utl/path-to-posix.mjs";
import { isFollowable } from "./module-classifiers.mjs";
import { resolve } from "./resolve.mjs";

// builtinModules does not expose all builtin modules for #reasons -
// see https://github.com/nodejs/node/issues/42785. In stead we could use
// isBuiltin, but that is not available in node 16.14, the lowest version
// of node dependency-cruiser currently supports. So we add the missing
// modules here.
// b.t.w. code is duplicated in resolve-amd.mjs
const REALLY_BUILTIN_MODULES = builtinModules.concat(["test", "node:test"]);

function addResolutionAttributes(
pBaseDirectory,
pModuleName,
pFileDirectory,
pResolveOptions
pResolveOptions,
) {
let lReturnValue = {};

if (builtinModules.includes(pModuleName)) {
if (REALLY_BUILTIN_MODULES.includes(pModuleName)) {
lReturnValue.coreModule = true;
} else {
try {
lReturnValue.resolved = pathToPosix(
relative(
pBaseDirectory,
resolve(pModuleName, pFileDirectory, pResolveOptions)
)
resolve(pModuleName, pFileDirectory, pResolveOptions),
),
);
lReturnValue.followable = isFollowable(
lReturnValue.resolved,
pResolveOptions
pResolveOptions,
);
} catch (pError) {
lReturnValue.couldNotResolve = true;
Expand All @@ -40,7 +48,7 @@ export default function resolveCommonJS(
pStrippedModuleName,
pBaseDirectory,
pFileDirectory,
pResolveOptions
pResolveOptions,
) {
return {
resolved: pStrippedModuleName,
Expand All @@ -51,7 +59,7 @@ export default function resolveCommonJS(
pBaseDirectory,
pStrippedModuleName,
pFileDirectory,
pResolveOptions
pResolveOptions,
),
};
}
40 changes: 40 additions & 0 deletions test/extract/resolve/index.general.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,46 @@ describe("[I] extract/resolve/index - general", () => {
});
});

it("resolves the 'test' core module as core module", () => {
expect(
resolve(
{
module: "test",
moduleSystem: "es6",
},
join(__dirname, "__mocks__"),
join(__dirname, "__mocks__", "resolve"),
{},
),
).to.deep.equal({
coreModule: true,
couldNotResolve: false,
dependencyTypes: ["core"],
followable: false,
resolved: "test",
});
});

it("resolves the 'node:test' core module as core module", () => {
expect(
resolve(
{
module: "node:test",
moduleSystem: "es6",
},
join(__dirname, "__mocks__"),
join(__dirname, "__mocks__", "resolve"),
{},
),
).to.deep.equal({
coreModule: true,
couldNotResolve: false,
dependencyTypes: ["core"],
followable: false,
resolved: "node:test",
});
});

it("resolves to the moduleName input (and depType 'unknown') when not resolvable on disk", () => {
expect(
resolve(
Expand Down

0 comments on commit 231a399

Please sign in to comment.