From ff39f60188b219ca368e463e4022be680ba7c73b Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 8 Nov 2021 16:11:37 +0100 Subject: [PATCH] fix(bazel): integration test rule not able to setup mappings for resolutions (#286) 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; + } +}