diff --git a/.changeset/polite-beds-doubt.md b/.changeset/polite-beds-doubt.md new file mode 100644 index 00000000000..5e3dc61f330 --- /dev/null +++ b/.changeset/polite-beds-doubt.md @@ -0,0 +1,5 @@ +--- +"hardhat": patch +--- + +Fixed the acceptance of relative paths to `node_modules` in npm remappings ([#8007](https://github.com/NomicFoundation/hardhat/pull/8007)) diff --git a/v-next/hardhat/src/internal/builtin-plugins/solidity/build-system/resolver/remapped-npm-packages-graph.ts b/v-next/hardhat/src/internal/builtin-plugins/solidity/build-system/resolver/remapped-npm-packages-graph.ts index 23e1819a7b2..cdebb6f825a 100644 --- a/v-next/hardhat/src/internal/builtin-plugins/solidity/build-system/resolver/remapped-npm-packages-graph.ts +++ b/v-next/hardhat/src/internal/builtin-plugins/solidity/build-system/resolver/remapped-npm-packages-graph.ts @@ -37,6 +37,32 @@ import { UserRemappingType } from "./types.js"; const HARDHAT_PROJECT_INPUT_SOURCE_NAME_ROOT = "project"; +/** + * Returns a normalized version of the path if it refers to a node_modules in + * the root directory (i.e. node_modules/...), or a `node_modules` directory + * in a parent directory (i.e. ../../node_modules/...). + * + * Otherwise returns `undefined`. + * + * @param pathToNormalize The path to normalize. + * @returns The normalized path (node_modules/...), or `undefined`. + */ +export function getNormalizeNodeModulesPath( + pathToNormalize: string, +): string | undefined { + if (pathToNormalize.startsWith("node_modules/")) { + return pathToNormalize; + } + + const normalized = path.posix.normalize(pathToNormalize); + + if (!/^(?:\.\.\/)*node_modules\//.test(normalized)) { + return undefined; + } + + return normalized.substring(normalized.indexOf("node_modules/")); +} + export type RemappingsReaderFunction = ( packageName: string, packageVersion: string, @@ -559,7 +585,7 @@ export class RemappedNpmPackagesGraphImplementation const prefix = remapping.prefix.endsWith("/") ? remapping.prefix : remapping.prefix + "/"; - const target = remapping.target.endsWith("/") + let target = remapping.target.endsWith("/") ? remapping.target : remapping.target + "/"; @@ -568,12 +594,13 @@ export class RemappedNpmPackagesGraphImplementation path.dirname(sourceOfTheRemapping), ); - // If the remapping's target starts with `node_modules/`, we treat - // it as trying to load an npm dependency, otherwise we treat it as a local + // If the remapping's target starts with `node_modules/`, we treat it as + // trying to load an npm dependency, otherwise we treat it as a local // remapping. + const normalizedNodeModulesTarget = getNormalizeNodeModulesPath(target); // Local remapping case - if (!target.startsWith("node_modules/")) { + if (normalizedNodeModulesTarget === undefined) { return { success: true, value: { @@ -593,6 +620,9 @@ export class RemappedNpmPackagesGraphImplementation source: sourceOfTheRemapping, }, }; + } else { + // We update the target to the normalized version + target = normalizedNodeModulesTarget; } // If we are here the remapping is a npm remapping. diff --git a/v-next/hardhat/test/internal/builtin-plugins/solidity/build-system/resolver/remapped-npm-packages-graph.ts b/v-next/hardhat/test/internal/builtin-plugins/solidity/build-system/resolver/remapped-npm-packages-graph.ts index 92264785feb..c7f7f1d09ab 100644 --- a/v-next/hardhat/test/internal/builtin-plugins/solidity/build-system/resolver/remapped-npm-packages-graph.ts +++ b/v-next/hardhat/test/internal/builtin-plugins/solidity/build-system/resolver/remapped-npm-packages-graph.ts @@ -23,6 +23,7 @@ import { } from "@nomicfoundation/hardhat-utils/fs"; import { + getNormalizeNodeModulesPath, isResolvedUserRemapping, RemappedNpmPackagesGraphImplementation, type RemappingsReaderFunction, @@ -1644,6 +1645,385 @@ contr:to-npm=node_modules/dep/contracts`, }); }); }); + + describe("Relative node_modules remappings", () => { + it("Should treat ../node_modules/ remappings as npm remappings", async () => { + const template: TestProjectTemplate = { + name: "relative-parent-node-modules", + version: "1.0.0", + files: { + "contracts/A.sol": `import "d/B.sol";`, + "remappings.txt": `d/=../node_modules/dep1/src/`, + }, + dependencies: { + dep1: { + name: "dep1", + version: "1.2.3", + files: { + "src/B.sol": ``, + }, + }, + }, + }; + + await using project = await useTestProjectTemplate(template); + const graph = await RemappedNpmPackagesGraphImplementation.create( + project.path, + ); + + const hhProjectPackage = graph.getHardhatProjectPackage(); + const contractsASol: ProjectResolvedFile = { + type: ResolvedFileType.PROJECT_FILE, + fsPath: path.join(project.path, "contracts/A.sol"), + content: { + text: `import "d/B.sol";`, + importPaths: ["d/B.sol"], + versionPragmas: [], + }, + inputSourceName: "project/contracts/A.sol", + package: hhProjectPackage, + }; + + // First, trigger loading without matching to see the UNRESOLVED_NPM form + await graph.selectBestUserRemapping(contractsASol, "nope"); + const userRemappings = graph.toJSON().userRemappingsPerPackage.project; + + assert.deepEqual(userRemappings, [ + { + type: "UNRESOLVED_NPM", + installationName: "dep1", + context: "project/", + prefix: "d/", + target: "node_modules/dep1/src/", + originalFormat: `d/=../node_modules/dep1/src/`, + source: path.join(project.path, "remappings.txt"), + }, + ]); + }); + + it("Should treat ./node_modules/ remappings as npm remappings", async () => { + const template: TestProjectTemplate = { + name: "relative-dot-node-modules", + version: "1.0.0", + files: { + "contracts/A.sol": `import "d/B.sol";`, + "remappings.txt": `d/=./node_modules/dep1/src/`, + }, + dependencies: { + dep1: { + name: "dep1", + version: "1.2.3", + files: { + "src/B.sol": ``, + }, + }, + }, + }; + + await using project = await useTestProjectTemplate(template); + const graph = await RemappedNpmPackagesGraphImplementation.create( + project.path, + ); + + const hhProjectPackage = graph.getHardhatProjectPackage(); + const contractsASol: ProjectResolvedFile = { + type: ResolvedFileType.PROJECT_FILE, + fsPath: path.join(project.path, "contracts/A.sol"), + content: { + text: `import "d/B.sol";`, + importPaths: ["d/B.sol"], + versionPragmas: [], + }, + inputSourceName: "project/contracts/A.sol", + package: hhProjectPackage, + }; + + await graph.selectBestUserRemapping(contractsASol, "nope"); + const userRemappings = graph.toJSON().userRemappingsPerPackage.project; + + assert.deepEqual(userRemappings, [ + { + type: "UNRESOLVED_NPM", + installationName: "dep1", + context: "project/", + prefix: "d/", + target: "node_modules/dep1/src/", + originalFormat: `d/=./node_modules/dep1/src/`, + source: path.join(project.path, "remappings.txt"), + }, + ]); + }); + + it("Should normalize ../../a/../node_modules/ correctly", async () => { + const template: TestProjectTemplate = { + name: "relative-complex-path-node-modules", + version: "1.0.0", + files: { + "contracts/A.sol": `import "d/B.sol";`, + "remappings.txt": `d/=../../a/../node_modules/dep1/src/`, + }, + dependencies: { + dep1: { + name: "dep1", + version: "1.2.3", + files: { + "src/B.sol": ``, + }, + }, + }, + }; + + await using project = await useTestProjectTemplate(template); + const graph = await RemappedNpmPackagesGraphImplementation.create( + project.path, + ); + + const hhProjectPackage = graph.getHardhatProjectPackage(); + const contractsASol: ProjectResolvedFile = { + type: ResolvedFileType.PROJECT_FILE, + fsPath: path.join(project.path, "contracts/A.sol"), + content: { + text: `import "d/B.sol";`, + importPaths: ["d/B.sol"], + versionPragmas: [], + }, + inputSourceName: "project/contracts/A.sol", + package: hhProjectPackage, + }; + + await graph.selectBestUserRemapping(contractsASol, "nope"); + const userRemappings = graph.toJSON().userRemappingsPerPackage.project; + + assert.deepEqual(userRemappings, [ + { + type: "UNRESOLVED_NPM", + installationName: "dep1", + context: "project/", + prefix: "d/", + target: "node_modules/dep1/src/", + originalFormat: `d/=../../a/../node_modules/dep1/src/`, + source: path.join(project.path, "remappings.txt"), + }, + ]); + }); + + it("Should treat ./something/node_modules/ as a LOCAL remapping (nested node_modules)", async () => { + const template: TestProjectTemplate = { + name: "nested-node-modules-local", + version: "1.0.0", + files: { + "contracts/A.sol": `import "d/B.sol";`, + "remappings.txt": `d/=./something/node_modules/dep1/`, + }, + }; + + await using project = await useTestProjectTemplate(template); + const graph = await RemappedNpmPackagesGraphImplementation.create( + project.path, + ); + + const hhProjectPackage = graph.getHardhatProjectPackage(); + const contractsASol: ProjectResolvedFile = { + type: ResolvedFileType.PROJECT_FILE, + fsPath: path.join(project.path, "contracts/A.sol"), + content: { + text: `import "d/B.sol";`, + importPaths: ["d/B.sol"], + versionPragmas: [], + }, + inputSourceName: "project/contracts/A.sol", + package: hhProjectPackage, + }; + + await graph.selectBestUserRemapping(contractsASol, "nope"); + const userRemappings = graph.toJSON().userRemappingsPerPackage.project; + + assert.deepEqual(userRemappings, [ + { + type: UserRemappingType.LOCAL, + context: "project/", + prefix: "d/", + target: "project/something/node_modules/dep1/", + originalFormat: `d/=./something/node_modules/dep1/`, + source: path.join(project.path, "remappings.txt"), + }, + ]); + }); + + it("Should resolve a relative npm remapping to the correct npm target", async () => { + const template: TestProjectTemplate = { + name: "resolve-relative-npm-remapping", + version: "1.0.0", + files: { + "contracts/A.sol": `import "d/B.sol";`, + "remappings.txt": `d/=../node_modules/dep1/src/`, + }, + dependencies: { + dep1: { + name: "dep1", + version: "1.2.3", + files: { + "src/B.sol": ``, + }, + }, + }, + }; + + await using project = await useTestProjectTemplate(template); + const graph = await RemappedNpmPackagesGraphImplementation.create( + project.path, + ); + + const hhProjectPackage = graph.getHardhatProjectPackage(); + const contractsASol: ProjectResolvedFile = { + type: ResolvedFileType.PROJECT_FILE, + fsPath: path.join(project.path, "contracts/A.sol"), + content: { + text: `import "d/B.sol";`, + importPaths: ["d/B.sol"], + versionPragmas: [], + }, + inputSourceName: "project/contracts/A.sol", + package: hhProjectPackage, + }; + + const result = await graph.selectBestUserRemapping( + contractsASol, + "d/B.sol", + ); + + assert.deepEqual(result, { + success: true, + value: { + type: UserRemappingType.NPM, + context: "project/", + prefix: "d/", + target: "npm/dep1@1.2.3/src/", + originalFormat: `d/=../node_modules/dep1/src/`, + source: path.join(project.path, "remappings.txt"), + targetNpmPackage: { + installationName: "dep1", + package: { + name: "dep1", + version: "1.2.3", + rootFsPath: path.join(project.path, "node_modules/dep1"), + inputSourceNameRoot: "npm/dep1@1.2.3", + exports: undefined, + }, + }, + }, + }); + }); + + it("Should skip identity remapping d/=../node_modules/d/ (same as d/=node_modules/d/)", async () => { + const template: TestProjectTemplate = { + name: "skip-relative-identity-remapping", + version: "1.0.0", + files: { + "contracts/A.sol": `import "d/B.sol";`, + "remappings.txt": `foo/=bar/ +d/=../node_modules/d/`, + }, + dependencies: { + d: { + name: "d", + version: "1.0.0", + files: { + "B.sol": ``, + }, + }, + }, + }; + + await using project = await useTestProjectTemplate(template); + const graph = await RemappedNpmPackagesGraphImplementation.create( + project.path, + ); + + const hhProjectPackage = graph.getHardhatProjectPackage(); + const contractsASol: ProjectResolvedFile = { + type: ResolvedFileType.PROJECT_FILE, + fsPath: path.join(project.path, "contracts/A.sol"), + content: { + text: `import "d/B.sol";`, + importPaths: ["d/B.sol"], + versionPragmas: [], + }, + inputSourceName: "project/contracts/A.sol", + package: hhProjectPackage, + }; + + await graph.selectBestUserRemapping(contractsASol, "nope"); + const userRemappings = graph.toJSON().userRemappingsPerPackage.project; + + // Only the local remapping should be present; the identity npm remapping is skipped + assert.deepEqual(userRemappings, [ + { + type: UserRemappingType.LOCAL, + context: "project/", + prefix: "foo/", + target: "project/bar/", + originalFormat: `foo/=bar/`, + source: path.join(project.path, "remappings.txt"), + }, + ]); + }); + }); + + describe("isResolvedUserRemapping", () => { + it("Should return true if the remapping is a resolved user remapping", () => { + const resolvedNpmRemapping: ResolvedUserRemapping = { + type: UserRemappingType.NPM, + context: "context", + prefix: "prefix", + target: "target", + originalFormat: "originalFormat", + source: "source", + targetNpmPackage: { + installationName: "installationName", + package: { + name: "name", + version: "version", + rootFsPath: "rootFsPath", + inputSourceNameRoot: "inputSourceNameRoot", + exports: undefined, + }, + }, + }; + + assert.ok( + isResolvedUserRemapping(resolvedNpmRemapping), + "Should be true for a resolved npm user remapping", + ); + + const localRemapping: ResolvedUserRemapping = { + type: UserRemappingType.LOCAL, + context: "context", + prefix: "prefix", + target: "target", + originalFormat: "originalFormat", + source: "source", + }; + + assert.ok( + isResolvedUserRemapping(localRemapping), + "Should be true for a local user remapping", + ); + }); + + it("Should return false for a plain remapping", () => { + const plainRemapping: Remapping = { + context: "context", + prefix: "prefix", + target: "target", + }; + + assert.ok( + !isResolvedUserRemapping(plainRemapping), + "Should be false for a plain remapping", + ); + }); + }); }); describe("readNpmPackageRemappings hook behavior", () => { @@ -2094,3 +2474,63 @@ describe("isResolvedUserRemapping", () => { ); }); }); + +describe("getNormalizeNodeModulesPath", () => { + it("Should return the target as-is when it starts with node_modules/", () => { + assert.equal( + getNormalizeNodeModulesPath("node_modules/dep1/src/"), + "node_modules/dep1/src/", + ); + }); + + it("Should normalize ../node_modules/ to node_modules/", () => { + assert.equal( + getNormalizeNodeModulesPath("../node_modules/dep1/src/"), + "node_modules/dep1/src/", + ); + }); + + it("Should normalize ../../node_modules/ to node_modules/", () => { + assert.equal( + getNormalizeNodeModulesPath("../../node_modules/dep1/src/"), + "node_modules/dep1/src/", + ); + }); + + it("Should normalize ./node_modules/ to node_modules/", () => { + assert.equal( + getNormalizeNodeModulesPath("./node_modules/dep1/src/"), + "node_modules/dep1/src/", + ); + }); + + it("Should normalize ../../a/../node_modules/ to node_modules/", () => { + assert.equal( + getNormalizeNodeModulesPath("../../a/../node_modules/dep1/src/"), + "node_modules/dep1/src/", + ); + }); + + it("Should return undefined for paths with node_modules nested inside another directory", () => { + assert.equal( + getNormalizeNodeModulesPath("./something/node_modules/dep1/"), + undefined, + ); + }); + + it("Should return undefined for paths with node_modules nested inside a parent directory", () => { + assert.equal( + getNormalizeNodeModulesPath("../something/node_modules/dep1/"), + undefined, + ); + }); + + it("Should return undefined for plain local paths", () => { + assert.equal(getNormalizeNodeModulesPath("src/contracts/"), undefined); + assert.equal(getNormalizeNodeModulesPath("lib/foo/"), undefined); + }); + + it("Should return undefined for npm/ paths", () => { + assert.equal(getNormalizeNodeModulesPath("npm/dep1@1.0.0/src/"), undefined); + }); +});