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/npm-env-vars.ts b/tests/legacy-cli/e2e/tests/commands/add/npm-env-vars.ts index f58c1dab247e..d35a7e0846d6 100644 --- a/tests/legacy-cli/e2e/tests/commands/add/npm-env-vars.ts +++ b/tests/legacy-cli/e2e/tests/commands/add/npm-env-vars.ts @@ -1,5 +1,4 @@ import { expectFileNotToExist, expectFileToExist } from '../../../utils/fs'; -import { getActivePackageManager } from '../../../utils/packages'; import { git, ng } from '../../../utils/process'; import { createNpmConfigForAuthentication, @@ -7,27 +6,21 @@ import { } from '../../../utils/registry'; export default async function () { - const packageManager = getActivePackageManager(); + // Yarn also supports NPM_CONFIG env variables. + // https://classic.yarnpkg.com/en/docs/envvars/ - if (packageManager === 'npm') { - const originalEnvironment = { ...process.env }; - try { - const command = ['add', '@angular/pwa', '--skip-confirmation']; + const command = ['add', '@angular/pwa', '--skip-confirmation']; - // Environment variables only - await expectFileNotToExist('src/manifest.webmanifest'); - setNpmEnvVarsForAuthentication(); - await ng(...command); - await expectFileToExist('src/manifest.webmanifest'); - await git('clean', '-dxf'); + // Environment variables only + await expectFileNotToExist('src/manifest.webmanifest'); + setNpmEnvVarsForAuthentication(); + 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'); - } finally { - process.env = originalEnvironment; - } - } + // 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/tests/commands/add/registry-option.ts b/tests/legacy-cli/e2e/tests/commands/add/registry-option.ts index 197d3bc6dfe0..f5888f788b62 100644 --- a/tests/legacy-cli/e2e/tests/commands/add/registry-option.ts +++ b/tests/legacy-cli/e2e/tests/commands/add/registry-option.ts @@ -10,21 +10,12 @@ export default async function () { await writeMultipleFiles({ '.npmrc': 'registry=http://127.0.0.1:9999', }); + // The environment variable has priority over the .npmrc - let originalRegistryVariable; - if (process.env['NPM_CONFIG_REGISTRY']) { - originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY']; - delete process.env['NPM_CONFIG_REGISTRY']; - } + delete process.env['NPM_CONFIG_REGISTRY']; - try { - await expectToFail(() => ng('add', '@angular/pwa', '--skip-confirmation')); + await expectToFail(() => ng('add', '@angular/pwa', '--skip-confirmation')); - await ng('add', `--registry=${testRegistry}`, '@angular/pwa', '--skip-confirmation'); - await expectFileToExist('src/manifest.webmanifest'); - } finally { - if (originalRegistryVariable) { - process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable; - } - } + await ng('add', `--registry=${testRegistry}`, '@angular/pwa', '--skip-confirmation'); + await expectFileToExist('src/manifest.webmanifest'); } diff --git a/tests/legacy-cli/e2e/tests/commands/add/secure-registry.ts b/tests/legacy-cli/e2e/tests/commands/add/secure-registry.ts index da514b8750bf..69da8d5845b9 100644 --- a/tests/legacy-cli/e2e/tests/commands/add/secure-registry.ts +++ b/tests/legacy-cli/e2e/tests/commands/add/secure-registry.ts @@ -5,38 +5,28 @@ import { expectToFail } from '../../../utils/utils'; export default async function () { // The environment variable has priority over the .npmrc - let originalRegistryVariable; - if (process.env['NPM_CONFIG_REGISTRY']) { - originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY']; - delete process.env['NPM_CONFIG_REGISTRY']; - } + delete process.env['NPM_CONFIG_REGISTRY']; - try { - const command = ['add', '@angular/pwa', '--skip-confirmation']; - await expectFileNotToExist('src/manifest.webmanifest'); + const command = ['add', '@angular/pwa', '--skip-confirmation']; + await expectFileNotToExist('src/manifest.webmanifest'); - // Works with unscoped registry authentication details - await createNpmConfigForAuthentication(false); - await ng(...command); - await expectFileToExist('src/manifest.webmanifest'); - await git('clean', '-dxf'); + // Works with unscoped registry authentication details + await createNpmConfigForAuthentication(false); + await ng(...command); + await expectFileToExist('src/manifest.webmanifest'); + await git('clean', '-dxf'); - // Works with scoped registry authentication details - await expectFileNotToExist('src/manifest.webmanifest'); + // Works with scoped registry authentication details + await expectFileNotToExist('src/manifest.webmanifest'); - await createNpmConfigForAuthentication(true); - await ng(...command); - await expectFileToExist('src/manifest.webmanifest'); + await createNpmConfigForAuthentication(true); + await ng(...command); + await expectFileToExist('src/manifest.webmanifest'); - // Invalid authentication token - await createNpmConfigForAuthentication(false, true); - await expectToFail(() => ng(...command)); + // Invalid authentication token + await createNpmConfigForAuthentication(false, true); + await expectToFail(() => ng(...command)); - await createNpmConfigForAuthentication(true, true); - await expectToFail(() => ng(...command)); - } finally { - if (originalRegistryVariable) { - process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable; - } - } + await createNpmConfigForAuthentication(true, true); + await expectToFail(() => ng(...command)); } 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/tests/update/update-secure-registry.ts b/tests/legacy-cli/e2e/tests/update/update-secure-registry.ts index 2c00be697f26..263892d1d01b 100644 --- a/tests/legacy-cli/e2e/tests/update/update-secure-registry.ts +++ b/tests/legacy-cli/e2e/tests/update/update-secure-registry.ts @@ -4,37 +4,26 @@ import { expectToFail } from '../../utils/utils'; export default async function () { // The environment variable has priority over the .npmrc - let originalRegistryVariable; - if (process.env['NPM_CONFIG_REGISTRY']) { - originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY']; - delete process.env['NPM_CONFIG_REGISTRY']; - } - + delete process.env['NPM_CONFIG_REGISTRY']; const worksMessage = 'We analyzed your package.json'; - try { - // Valid authentication token - await createNpmConfigForAuthentication(false); - const { stdout: stdout1 } = await ng('update'); - if (!stdout1.includes(worksMessage)) { - throw new Error(`Expected stdout to contain "${worksMessage}"`); - } + // Valid authentication token + await createNpmConfigForAuthentication(false); + const { stdout: stdout1 } = await ng('update'); + if (!stdout1.includes(worksMessage)) { + throw new Error(`Expected stdout to contain "${worksMessage}"`); + } - await createNpmConfigForAuthentication(true); - const { stdout: stdout2 } = await ng('update'); - if (!stdout2.includes(worksMessage)) { - throw new Error(`Expected stdout to contain "${worksMessage}"`); - } + await createNpmConfigForAuthentication(true); + const { stdout: stdout2 } = await ng('update'); + if (!stdout2.includes(worksMessage)) { + throw new Error(`Expected stdout to contain "${worksMessage}"`); + } - // Invalid authentication token - await createNpmConfigForAuthentication(false, true); - await expectToFail(() => ng('update')); + // Invalid authentication token + await createNpmConfigForAuthentication(false, true); + await expectToFail(() => ng('update')); - await createNpmConfigForAuthentication(true, true); - await expectToFail(() => ng('update')); - } finally { - if (originalRegistryVariable) { - process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable; - } - } + await createNpmConfigForAuthentication(true, true); + await expectToFail(() => ng('update')); } diff --git a/tests/legacy-cli/e2e/utils/packages.ts b/tests/legacy-cli/e2e/utils/packages.ts index a8416c9ae24c..2f7fdef6f271 100644 --- a/tests/legacy-cli/e2e/utils/packages.ts +++ b/tests/legacy-cli/e2e/utils/packages.ts @@ -53,8 +53,7 @@ export async function setRegistry(useTestRegistry: boolean): Promise { // Safe to set a user configuration on CI await npm('config', 'set', 'registry', url); } else { - // Yarn does not use the environment variable so an .npmrc file is also required - await writeFile('.npmrc', `registry=${url}`); + // Yarn supports both `NPM_CONFIG_REGISTRY` and `YARN_REGISTRY`. process.env['NPM_CONFIG_REGISTRY'] = url; } } 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..6fdc31ec6bdf 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 @@ -169,16 +173,19 @@ testsToRun .then(() => { // If we're not in a setup, change the directory back to where it was before the test. // This allows tests to chdir without worrying about keeping the original directory. - if (allSetups.indexOf(relativeName) == -1 && previousDir) { + if (!allSetups.includes(relativeName) && previousDir) { process.chdir(previousDir); + + // Restore env variables before each test. + console.log(' Restoring original environment variables...'); + process.env = originalEnvVariables; } }) .then(() => { // Only clean after a real test, not a setup step. Also skip cleaning if the test // requested an exception. - if (allSetups.indexOf(relativeName) == -1 && clean) { + if (!allSetups.includes(relativeName) && clean) { logStack.push(new logging.NullLogger()); - return gitClean().then( () => logStack.pop(), (err) => {