Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(resolve): hands over preserveSymlink processing to EHR #865

Merged
merged 2 commits into from
Nov 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 25 additions & 30 deletions src/extract/resolve/index.mjs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { realpathSync } from "node:fs";
import { extname, resolve as path_resolve, relative } from "node:path";
import monkeyPatchedModule from "node:module";
import { isRelativeModuleName } from "./module-classifiers.mjs";
Expand Down Expand Up @@ -54,7 +53,7 @@ function canBeResolvedToTsVariant(pModuleName) {
function isTypeScriptIshExtension(pModuleName) {
return [".ts", ".tsx", ".cts", ".mts"].includes(extname(pModuleName));
}
function resolveYarnVirtual(pPath) {
function resolveYarnVirtual(pBaseDirectory, pPath) {
const pnpAPI = (monkeyPatchedModule?.findPnpApi ?? (() => false))(pPath);

// the pnp api only works in plug'n play environments, and resolveVirtual
Expand All @@ -63,7 +62,15 @@ function resolveYarnVirtual(pPath) {
// cover it in a separate integration test.
/* c8 ignore start */
if (pnpAPI && (pnpAPI?.VERSIONS?.resolveVirtual ?? 0) === 1) {
return pnpAPI.resolveVirtual(path_resolve(pPath)) || pPath;
// resolveVirtual takes absolute paths, hence the path.resolve:
const lResolvedAbsolute = path_resolve(pBaseDirectory, pPath);
const lResolvedVirtual = pnpAPI.resolveVirtual(lResolvedAbsolute);
if (lResolvedVirtual) {
const lResolvedRelative = relative(pBaseDirectory, lResolvedVirtual);
// in win32 environments resolveVirtual might return win32 paths,
// so we have to convert them to posix paths again
return pathToPosix(lResolvedRelative);
}
}
/* c8 ignore stop */
return pPath;
Expand Down Expand Up @@ -193,33 +200,21 @@ export default function resolve(
),
};

if (
!pResolveOptions.symlinks &&
!lResolvedDependency.coreModule &&
!lResolvedDependency.couldNotResolve
) {
try {
lResolvedDependency.resolved = pathToPosix(
relative(
pBaseDirectory,
realpathSync(
resolveYarnVirtual(
path_resolve(
pBaseDirectory,
/* enhanced-resolve inserts a NULL character in front of any `#`
character. This wonky replace undoes that so the filename
again corresponds with a real file on disk
*/
// eslint-disable-next-line no-control-regex
lResolvedDependency.resolved.replace(/\u0000#/g, "#"),
),
),
),
),
);
} catch (pError) {
lResolvedDependency.couldNotResolve = true;
}
if (!lResolvedDependency.coreModule && !lResolvedDependency.couldNotResolve) {
// enhanced-resolve inserts a NULL character in front of any `#` character.
// This wonky replace corrects that that so the filename again corresponds
// with a real file on disk
const lResolvedEHRCorrected = lResolvedDependency.resolved.replace(
// eslint-disable-next-line no-control-regex
/\u0000#/g,
"#",
);
const lResolvedYarnVirtual = resolveYarnVirtual(
pBaseDirectory,
lResolvedEHRCorrected,
);

lResolvedDependency.resolved = lResolvedYarnVirtual;
}
return lResolvedDependency;
}
16 changes: 4 additions & 12 deletions src/extract/resolve/resolve.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,12 @@ import pathToPosix from "#utl/path-to-posix.mjs";
let gResolvers = {};
let gInitialized = {};

function init(pResolveOptions, pCachingContext) {
if (!gInitialized[pCachingContext] || pResolveOptions.bustTheCache) {
function init(pEHResolveOptions, pCachingContext) {
if (!gInitialized[pCachingContext] || pEHResolveOptions.bustTheCache) {
// assuming the cached file system here
pResolveOptions.fileSystem.purge();
pEHResolveOptions.fileSystem.purge();
gResolvers[pCachingContext] =
enhancedResolve.ResolverFactory.createResolver({
...pResolveOptions,
// we're doing that ourselves for now. We can't set this in
// 'normalize' because we actively use resolveOptions.symlinks
// with our own symlink resolution thing, so we need to override
// it here locally so even when it is passed as true we skip
// ehr's capabilities in this and still do it ourselves.
symlinks: false,
});
enhancedResolve.ResolverFactory.createResolver(pEHResolveOptions);
/* eslint security/detect-object-injection:0 */
gInitialized[pCachingContext] = true;
}
Expand Down
22 changes: 7 additions & 15 deletions src/main/resolve-options/normalize.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,7 @@ import {
const DEFAULT_CACHE_DURATION = 4000;
/** @type {Partial<import("../../../types/dependency-cruiser").IResolveOptions>} */
const DEFAULT_RESOLVE_OPTIONS = {
// for later: check semantics of enhanced-resolve symlinks and
// node's preserveSymlinks. They seem to be
// symlink === !preserveSymlinks - but using it that way
// breaks backwards compatibility
//
// Someday we'll rely on this and remove the code that manually
// does this in extract/resolve/index.js
symlinks: false,
symlinks: true,
// if a webpack config overrides extensions, there's probably
// good cause. The scannableExtensions are an educated guess
// anyway, that works well in most circumstances.
Expand Down Expand Up @@ -145,13 +138,12 @@ export default async function normalizeResolveOptions(
// eslint-disable-next-line no-return-await
return await compileResolveOptions(
{
/*
for later: check semantics of enhanced-resolve symlinks and
node's preserveSymlinks. They seem to be
symlink === !preserveSymlinks - but using it that way
breaks backwards compatibility
*/
symlinks: pOptions?.preserveSymlinks ?? null,
// EnhancedResolve's symlinks:
// - true => symlinks are followed (vv)
// node's --preserve-symlinks:
// - true => symlinks are NOT followed (vv)
// => symlinks = !preserveSymlinks
symlinks: !pOptions?.preserveSymlinks,
tsConfig: pOptions?.ruleSet?.options?.tsConfig?.fileName ?? null,

/* squirrel the externalModuleResolutionStrategy and combinedDependencies
Expand Down
8 changes: 4 additions & 4 deletions test/main/resolve-options/normalize.spec.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe("[I] main/resolve-options/normalize", () => {
);

equal(Object.keys(lNormalizedOptions).length, lDefaultNoOfResolveOptions);
equal(lNormalizedOptions.symlinks, false);
equal(lNormalizedOptions.symlinks, true);
equal(lNormalizedOptions.tsConfig, null);
equal(lNormalizedOptions.combinedDependencies, false);
ok(lNormalizedOptions.hasOwnProperty("extensions"));
Expand All @@ -39,7 +39,7 @@ describe("[I] main/resolve-options/normalize", () => {
);

equal(Object.keys(lNormalizedOptions).length, lDefaultNoOfResolveOptions);
equal(lNormalizedOptions.symlinks, false);
equal(lNormalizedOptions.symlinks, true);
equal(lNormalizedOptions.tsConfig, null);
equal(lNormalizedOptions.combinedDependencies, false);
ok(lNormalizedOptions.hasOwnProperty("extensions"));
Expand All @@ -61,7 +61,7 @@ describe("[I] main/resolve-options/normalize", () => {
Object.keys(lNormalizedOptions).length,
lDefaultNoOfResolveOptions + 1,
);
equal(lNormalizedOptions.symlinks, false);
equal(lNormalizedOptions.symlinks, true);
equal(lNormalizedOptions.tsConfig, TEST_TSCONFIG);
equal(lNormalizedOptions.combinedDependencies, false);
ok(lNormalizedOptions.hasOwnProperty("extensions"));
Expand All @@ -83,7 +83,7 @@ describe("[I] main/resolve-options/normalize", () => {
Object.keys(lNormalizedOptions).length,
lDefaultNoOfResolveOptions + 1,
);
equal(lNormalizedOptions.symlinks, false);
equal(lNormalizedOptions.symlinks, true);
equal(lNormalizedOptions.tsConfig, TEST_TSCONFIG);
equal(lNormalizedOptions.combinedDependencies, false);
ok(lNormalizedOptions.hasOwnProperty("extensions"));
Expand Down