From e05e4b0ae3373871952a0f42b3d062600175c490 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Sun, 7 Nov 2021 13:40:40 +0100 Subject: [PATCH] fix(bazel): integration test rule not able to setup mappings for resolutions Fixes that the Bazel integration test rule is not able to setup mappings for Yarn resolutions because the record keys do not exactly match to a package name, but rather follow certain allowed patterns, like `**/`. --- bazel/integration/test_runner/package_json.ts | 59 ++++++++++++++++--- .../tests/package_mappings/package.json | 11 ++++ .../tests/package_mappings/test.mjs | 11 ++++ 3 files changed, 73 insertions(+), 8 deletions(-) diff --git a/bazel/integration/test_runner/package_json.ts b/bazel/integration/test_runner/package_json.ts index a83f39f45..57b82537a 100644 --- a/bazel/integration/test_runner/package_json.ts +++ b/bazel/integration/test_runner/package_json.ts @@ -49,10 +49,18 @@ export function updateMappingsForPackageJson( ): PackageJson { const newPackageJson = {...packageJson}; - updateMappingForRecord(newPackageJson.dependencies ?? {}, mappings); - updateMappingForRecord(newPackageJson.devDependencies ?? {}, mappings); - updateMappingForRecord(newPackageJson.optionalDependencies ?? {}, mappings); - updateMappingForRecord(newPackageJson.resolutions ?? {}, mappings); + updateMappingForRecord(newPackageJson, 'dependencies', mappings); + updateMappingForRecord(newPackageJson, 'devDependencies', mappings); + updateMappingForRecord(newPackageJson, 'optionalDependencies', mappings); + // The object for Yarn resolutions will not directly match with the mapping keys + // specified here, as resolutions usually follow a format as followed: + // 1. `**/` + // 2. `/**/` + // 3. `` + // More details here: https://classic.yarnpkg.com/lang/en/docs/selective-version-resolutions/. + // We pass a regular expression which matches the `` so that the mappings can + // be applied for resolutions as well. + updateMappingForRecord(newPackageJson, 'resolutions', mappings, /([^/]+)$/); return newPackageJson; } @@ -64,15 +72,31 @@ export function updateMappingsForPackageJson( * @throws An error if there is a dependency entry referring to a local file. Such * entries should not use `file:` but instead configure a mapping through Bazel. */ -function updateMappingForRecord(record: DependencyRecord, mappings: PackageMappings) { - for (const [pkgName, value] of Object.entries(record)) { +function updateMappingForRecord( + pkgJson: PackageJson, + recordName: keyof PackageJson, + mappings: PackageMappings, + nameMatchRegex?: RegExp, +) { + const record = pkgJson[recordName] ?? {}; + + for (const [entryKey, value] of Object.entries(record)) { + const pkgName = getPackageNameFromDependencyEntry(entryKey, nameMatchRegex); + + if (pkgName === null) { + throw Error(`Could not determine package name for "${recordName}" entry: ${entryKey}.`); + } + + // Print the resolved package name to ease debugging when packages are not mapped properly. + debug(`updateMappingForRecord: Resolved "${recordName}@${entryKey}" to package: ${pkgName}`); + const mappedAbsolutePath = mappings[pkgName]; // If the value of the dependency entry is referring to a local file, then we report // an error as this is likely a missing mapping that should be set up through Bazel. if (mappedAbsolutePath === undefined && value.startsWith(`file:`)) { throw Error( - `Unexpected dependency entry for: ${pkgName}, pointing to: ${value}.` + + `Unexpected dependency entry for: ${entryKey}, pointing to: ${value}. ` + `Instead, configure the mapping through the integration test Bazel target.`, ); } @@ -82,6 +106,25 @@ function updateMappingForRecord(record: DependencyRecord, mappings: PackageMappi continue; } - record[pkgName] = mappedAbsolutePath; + record[entryKey] = mappedAbsolutePath; + } +} + +/** + * Gets the package name from a dependency record entry. + * + * @param entryKey Key of the dependency record entry. + * @param nameMatchRegex Optional regular expression that can be specified to match the package + * name in a dependency entry. This is useful for e.g. Yarn resolutions using patterns. + * The first capturing group is expected to return the matched package name. + */ +function getPackageNameFromDependencyEntry( + entryKey: string, + nameMatchRegex?: RegExp, +): string | null { + if (nameMatchRegex) { + const matches = entryKey.match(nameMatchRegex); + return matches === null ? null : matches[1]; } + return entryKey; } diff --git a/bazel/integration/tests/package_mappings/package.json b/bazel/integration/tests/package_mappings/package.json index c23866f45..c0d51a775 100644 --- a/bazel/integration/tests/package_mappings/package.json +++ b/bazel/integration/tests/package_mappings/package.json @@ -5,5 +5,16 @@ "license": "MIT", "dependencies": { "fake_pkg": "0.0.0" + }, + "devDependencies": { + "fake_pkg": "0.0.0" + }, + "optionalDependencies": { + "fake_pkg": "0.0.0" + }, + "resolutions": { + "**/fake_pkg": "0.0.0", + "some_pkg/**/fake_pkg": "0.0.0", + "fake_pkg": "0.0.0" } } diff --git a/bazel/integration/tests/package_mappings/test.mjs b/bazel/integration/tests/package_mappings/test.mjs index 96c111800..aa05483ab 100644 --- a/bazel/integration/tests/package_mappings/test.mjs +++ b/bazel/integration/tests/package_mappings/test.mjs @@ -1,4 +1,5 @@ import fakePkg from 'fake_pkg'; +import fs from 'fs'; // Sanity check that the installed package matches the one we have // built from source using `pkg_npm`. @@ -6,3 +7,13 @@ if (fakePkg !== 'This is a fake package!') { console.error('Fake package is not matching with locally-built one.'); process.exitCode = 1; } + +const pkgJson = JSON.parse(fs.readFileSync('./package.json', 'utf8')); +const recordsToCheck = ['dependencies', 'devDependencies', 'optionalDependencies', 'resolutions']; + +for (const recordName of recordsToCheck) { + if (Object.values(pkgJson[recordName]).includes('0.0.0')) { + console.error(`The "${recordName}" field has not been replaced with mapped archives.`); + process.exitCode = 1; + } +}