From c1eddbdc98631fdfff287ce566d79ed43b601e0f Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Wed, 21 Jul 2021 09:36:30 +0200 Subject: [PATCH] fix(@angular/cli): handle `YARN_` environment variables during `ng update` and `ng add` With this change we handle yarn specific environment variables during `ng update` and `ng add`. This is a follow up of #21297 --- .../angular/cli/utilities/package-metadata.ts | 44 +++++++++---------- .../e2e/tests/commands/add/yarn-env-vars.ts | 29 ++++++++++++ tests/legacy-cli/e2e/utils/registry.ts | 16 +++++-- tests/legacy-cli/e2e_runner.ts | 9 ++++ 4 files changed, 70 insertions(+), 28 deletions(-) create mode 100644 tests/legacy-cli/e2e/tests/commands/add/yarn-env-vars.ts diff --git a/packages/angular/cli/utilities/package-metadata.ts b/packages/angular/cli/utilities/package-metadata.ts index c96ad8cdc5be..4b709e21e634 100644 --- a/packages/angular/cli/utilities/package-metadata.ts +++ b/packages/angular/cli/utilities/package-metadata.ts @@ -131,7 +131,7 @@ function readOptions( logger.info(`Locating potential ${baseFilename} files:`); } - let options: PackageManagerOptions = {}; + let rcOptions: PackageManagerOptions = {}; for (const location of [...defaultConfigLocations, ...projectConfigLocations]) { if (existsSync(location)) { if (showPotentials) { @@ -143,25 +143,33 @@ function readOptions( // See: https://github.com/npm/npm-registry-fetch/blob/ebddbe78a5f67118c1f7af2e02c8a22bcaf9e850/index.js#L99-L126 const rcConfig: PackageManagerOptions = yarn ? lockfile.parse(data) : ini.parse(data); - options = normalizeOptions(rcConfig, location); + rcOptions = normalizeOptions(rcConfig, location); } } + const envVariablesOptions: PackageManagerOptions = {}; for (const [key, value] of Object.entries(process.env)) { - if (!value || !key.toLowerCase().startsWith('npm_config_')) { + if (!value) { continue; } - const normalizedName = key - .substr(11) - .replace(/(?!^)_/g, '-') // don't replace _ at the start of the key - .toLowerCase(); - options[normalizedName] = value; - } + let normalizedName = key.toLowerCase(); + if (normalizedName.startsWith('npm_config_')) { + normalizedName = normalizedName.substring(11); + } else if (yarn && normalizedName.startsWith('yarn_')) { + normalizedName = normalizedName.substring(5); + } else { + continue; + } - options = normalizeOptions(options); + normalizedName = normalizedName.replace(/(?!^)_/g, '-'); // don't replace _ at the start of the key.s + envVariablesOptions[normalizedName] = value; + } - return options; + return { + ...rcOptions, + ...normalizeOptions(envVariablesOptions), + }; } function normalizeOptions( @@ -302,7 +310,6 @@ export async function fetchPackageManifest( } = {}, ): Promise { const { usingYarn = false, verbose = false, registry } = options; - ensureNpmrc(logger, usingYarn, verbose); const response = await pacote.manifest(name, { @@ -329,18 +336,7 @@ export function getNpmPackageJson( } const { usingYarn = false, verbose = false, registry } = options; - - if (!npmrc) { - try { - npmrc = readOptions(logger, false, verbose); - } catch {} - - if (usingYarn) { - try { - npmrc = { ...npmrc, ...readOptions(logger, true, verbose) }; - } catch {} - } - } + ensureNpmrc(logger, usingYarn, verbose); const resultPromise: Promise = pacote.packument(packageName, { fullMetadata: true, diff --git a/tests/legacy-cli/e2e/tests/commands/add/yarn-env-vars.ts b/tests/legacy-cli/e2e/tests/commands/add/yarn-env-vars.ts new file mode 100644 index 000000000000..67ffdbdc9b09 --- /dev/null +++ b/tests/legacy-cli/e2e/tests/commands/add/yarn-env-vars.ts @@ -0,0 +1,29 @@ +import { expectFileNotToExist, expectFileToExist } from '../../../utils/fs'; +import { getActivePackageManager } from '../../../utils/packages'; +import { git, ng } from '../../../utils/process'; +import { + createNpmConfigForAuthentication, + setNpmEnvVarsForAuthentication, +} from '../../../utils/registry'; + +export default async function () { + // Yarn specific test that tests YARN_ env variables. + // https://classic.yarnpkg.com/en/docs/envvars/ + if (getActivePackageManager() !== 'yarn') { + return; + } + const command = ['add', '@angular/pwa', '--skip-confirmation']; + + // Environment variables only + await expectFileNotToExist('src/manifest.webmanifest'); + setNpmEnvVarsForAuthentication(false, true); + await ng(...command); + await expectFileToExist('src/manifest.webmanifest'); + await git('clean', '-dxf'); + + // Mix of config file and env vars works + await expectFileNotToExist('src/manifest.webmanifest'); + await createNpmConfigForAuthentication(false, true); + await ng(...command); + await expectFileToExist('src/manifest.webmanifest'); +} diff --git a/tests/legacy-cli/e2e/utils/registry.ts b/tests/legacy-cli/e2e/utils/registry.ts index a22c75858132..4395e5b1cae7 100644 --- a/tests/legacy-cli/e2e/utils/registry.ts +++ b/tests/legacy-cli/e2e/utils/registry.ts @@ -61,10 +61,18 @@ export function createNpmConfigForAuthentication( export function setNpmEnvVarsForAuthentication( /** When true, an incorrect token is used. Use this to validate authentication failures. */ invalidToken = false, + /** When true, `YARN_REGISTRY` is used instead of `NPM_CONFIG_REGISTRY`. */ + useYarnEnvVariable = false, ): void { - const token = invalidToken ? `invalid=` : VALID_TOKEN; - const registry = SECURE_REGISTRY; + delete process.env['YARN_REGISTRY']; + delete process.env['NPM_CONFIG_REGISTRY']; + + const registryKey = useYarnEnvVariable ? 'YARN_REGISTRY' : 'NPM_CONFIG_REGISTRY'; + process.env[registryKey] = `http:${SECURE_REGISTRY}`; + + process.env['NPM_CONFIG__AUTH'] = invalidToken ? `invalid=` : VALID_TOKEN; - process.env['NPM_CONFIG_REGISTRY'] = `http:${registry}`; - process.env['NPM_CONFIG__AUTH'] = token; + // Needed for verdaccio when used with yarn + // https://verdaccio.org/docs/en/cli-registry#yarn + process.env['NPM_CONFIG_ALWAYS_AUTH'] = 'true'; } diff --git a/tests/legacy-cli/e2e_runner.ts b/tests/legacy-cli/e2e_runner.ts index 04bb84f88ce1..3f88fa6e44ef 100644 --- a/tests/legacy-cli/e2e_runner.ts +++ b/tests/legacy-cli/e2e_runner.ts @@ -141,6 +141,10 @@ testsToRun const start = +new Date(); const module = require(absoluteName); + const originalEnvVariables = { + ...process.env, + }; + const fn: (skipClean?: () => void) => Promise | void = typeof module == 'function' ? module @@ -188,6 +192,11 @@ testsToRun ); } }) + .finally(() => { + // Restore env variables after each test. + console.log(' Restoring original environment variables...'); + process.env = originalEnvVariables; + }) .then( () => printFooter(currentFileName, start), (err) => {