From c4944fc44165631c3536703f5d552da7ddb11fc8 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Wed, 16 Jun 2021 13:04:20 +0200 Subject: [PATCH 1/3] refactor(@angular/cli): remove duplicate npmrc logic --- .../src/commands/update/schematic/index.ts | 7 +- .../update/schematic/npm-package-json.ts | 28 --- .../cli/src/commands/update/schematic/npm.ts | 166 ------------------ .../schematic => utilities}/package-json.ts | 0 .../angular/cli/utilities/package-metadata.ts | 127 ++++++++++---- 5 files changed, 95 insertions(+), 233 deletions(-) delete mode 100644 packages/angular/cli/src/commands/update/schematic/npm-package-json.ts delete mode 100644 packages/angular/cli/src/commands/update/schematic/npm.ts rename packages/angular/cli/{src/commands/update/schematic => utilities}/package-json.ts (100%) diff --git a/packages/angular/cli/src/commands/update/schematic/index.ts b/packages/angular/cli/src/commands/update/schematic/index.ts index 56ba9ee8be47..528c501ddb3c 100644 --- a/packages/angular/cli/src/commands/update/schematic/index.ts +++ b/packages/angular/cli/src/commands/update/schematic/index.ts @@ -10,9 +10,8 @@ import { logging, tags } from '@angular-devkit/core'; import { Rule, SchematicContext, SchematicsException, Tree } from '@angular-devkit/schematics'; import * as npa from 'npm-package-arg'; import * as semver from 'semver'; -import { getNpmPackageJson } from './npm'; -import { NpmRepositoryPackageJson } from './npm-package-json'; -import { Dependency, JsonSchemaForNpmPackageJsonFiles } from './package-json'; +import { Dependency, JsonSchemaForNpmPackageJsonFiles } from '../../../../utilities/package-json'; +import { NpmRepositoryPackageJson, getNpmPackageJson } from '../../../../utilities/package-metadata'; import { Schema as UpdateSchema } from './schema'; type VersionRange = string & { __VERSION_RANGE: void }; @@ -818,7 +817,7 @@ export default function (options: UpdateSchema): Rule { const allPackageMetadata = await Promise.all( Array.from(npmDeps.keys()).map((depName) => getNpmPackageJson(depName, logger, { - registryUrl: options.registry, + registry: options.registry, usingYarn, verbose: options.verbose, }), diff --git a/packages/angular/cli/src/commands/update/schematic/npm-package-json.ts b/packages/angular/cli/src/commands/update/schematic/npm-package-json.ts deleted file mode 100644 index 42bac6aeca53..000000000000 --- a/packages/angular/cli/src/commands/update/schematic/npm-package-json.ts +++ /dev/null @@ -1,28 +0,0 @@ -/** - * @license - * Copyright Google LLC All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -import { JsonSchemaForNpmPackageJsonFiles } from './package-json'; - -export interface NpmRepositoryPackageJson { - name: string; - requestedName: string; - description: string; - - 'dist-tags': { - [name: string]: string; - }; - versions: { - [version: string]: JsonSchemaForNpmPackageJsonFiles; - }; - time: { - modified: string; - created: string; - - [version: string]: string; - }; -} diff --git a/packages/angular/cli/src/commands/update/schematic/npm.ts b/packages/angular/cli/src/commands/update/schematic/npm.ts deleted file mode 100644 index 1eb084c90a1b..000000000000 --- a/packages/angular/cli/src/commands/update/schematic/npm.ts +++ /dev/null @@ -1,166 +0,0 @@ -/** - * @license - * Copyright Google LLC All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ - -import { logging } from '@angular-devkit/core'; -import { existsSync, readFileSync } from 'fs'; -import { homedir } from 'os'; -import * as path from 'path'; -import { NpmRepositoryPackageJson } from './npm-package-json'; - -const lockfile = require('@yarnpkg/lockfile'); -const ini = require('ini'); -const pacote = require('pacote'); - -type PackageManagerOptions = Record; - -const npmPackageJsonCache = new Map>>(); -let npmrc: PackageManagerOptions; - -function readOptions( - logger: logging.LoggerApi, - yarn = false, - showPotentials = false, -): PackageManagerOptions { - const cwd = process.cwd(); - const baseFilename = yarn ? 'yarnrc' : 'npmrc'; - const dotFilename = '.' + baseFilename; - - let globalPrefix: string; - if (process.env.PREFIX) { - globalPrefix = process.env.PREFIX; - } else { - globalPrefix = path.dirname(process.execPath); - if (process.platform !== 'win32') { - globalPrefix = path.dirname(globalPrefix); - } - } - - const defaultConfigLocations = [ - path.join(globalPrefix, 'etc', baseFilename), - path.join(homedir(), dotFilename), - ]; - - const projectConfigLocations: string[] = [path.join(cwd, dotFilename)]; - const root = path.parse(cwd).root; - for (let curDir = path.dirname(cwd); curDir && curDir !== root; curDir = path.dirname(curDir)) { - projectConfigLocations.unshift(path.join(curDir, dotFilename)); - } - - if (showPotentials) { - logger.info(`Locating potential ${baseFilename} files:`); - } - - const options: PackageManagerOptions = {}; - for (const location of [...defaultConfigLocations, ...projectConfigLocations]) { - if (existsSync(location)) { - if (showPotentials) { - logger.info(`Trying '${location}'...found.`); - } - - const data = readFileSync(location, 'utf8'); - // Normalize RC options that are needed by 'npm-registry-fetch'. - // See: https://github.com/npm/npm-registry-fetch/blob/ebddbe78a5f67118c1f7af2e02c8a22bcaf9e850/index.js#L99-L126 - const rcConfig: PackageManagerOptions = yarn ? lockfile.parse(data) : ini.parse(data); - for (const [key, value] of Object.entries(rcConfig)) { - switch (key) { - case 'noproxy': - case 'no-proxy': - options['noProxy'] = value; - break; - case 'maxsockets': - options['maxSockets'] = value; - break; - case 'https-proxy': - case 'proxy': - options['proxy'] = value; - break; - case 'strict-ssl': - options['strictSSL'] = value; - break; - case 'local-address': - options['localAddress'] = value; - break; - case 'cafile': - if (typeof value === 'string') { - const cafile = path.resolve(path.dirname(location), value); - try { - options['ca'] = readFileSync(cafile, 'utf8').replace(/\r?\n/g, '\n'); - } catch {} - } - break; - default: - options[key] = value; - break; - } - } - } else if (showPotentials) { - logger.info(`Trying '${location}'...not found.`); - } - } - - // Substitute any environment variable references - for (const key in options) { - const value = options[key]; - if (typeof value === 'string') { - options[key] = value.replace(/\$\{([^\}]+)\}/, (_, name) => process.env[name] || ''); - } - } - - return options; -} - -/** - * Get the NPM repository's package.json for a package. This is p - * @param {string} packageName The package name to fetch. - * @param {string} registryUrl The NPM Registry URL to use. - * @param {LoggerApi} logger A logger instance to log debug information. - * @returns An observable that will put the pacakge.json content. - * @private - */ -export function getNpmPackageJson( - packageName: string, - logger: logging.LoggerApi, - options?: { - registryUrl?: string; - usingYarn?: boolean; - verbose?: boolean; - }, -): Promise> { - const cachedResponse = npmPackageJsonCache.get(packageName); - if (cachedResponse) { - return cachedResponse; - } - - if (!npmrc) { - try { - npmrc = readOptions(logger, false, options && options.verbose); - } catch {} - - if (options && options.usingYarn) { - try { - npmrc = { ...npmrc, ...readOptions(logger, true, options && options.verbose) }; - } catch {} - } - } - - const resultPromise: Promise = pacote.packument(packageName, { - fullMetadata: true, - ...npmrc, - ...(options && options.registryUrl ? { registry: options.registryUrl } : {}), - }); - - // TODO: find some way to test this - const response = resultPromise.catch((err) => { - logger.warn(err.message || err); - - return { requestedName: packageName }; - }); - npmPackageJsonCache.set(packageName, response); - - return response; -} diff --git a/packages/angular/cli/src/commands/update/schematic/package-json.ts b/packages/angular/cli/utilities/package-json.ts similarity index 100% rename from packages/angular/cli/src/commands/update/schematic/package-json.ts rename to packages/angular/cli/utilities/package-json.ts diff --git a/packages/angular/cli/utilities/package-metadata.ts b/packages/angular/cli/utilities/package-metadata.ts index 129cca59d6db..fe7a42b91254 100644 --- a/packages/angular/cli/utilities/package-metadata.ts +++ b/packages/angular/cli/utilities/package-metadata.ts @@ -10,13 +10,31 @@ import { logging } from '@angular-devkit/core'; import { existsSync, readFileSync } from 'fs'; import { homedir } from 'os'; import * as path from 'path'; +import { JsonSchemaForNpmPackageJsonFiles } from './package-json'; const lockfile = require('@yarnpkg/lockfile'); const ini = require('ini'); const pacote = require('pacote'); -export interface PackageDependencies { - [dependency: string]: string; +const npmPackageJsonCache = new Map>>(); + +export interface NpmRepositoryPackageJson { + name: string; + requestedName: string; + description: string; + + 'dist-tags': { + [name: string]: string; + }; + versions: { + [version: string]: JsonSchemaForNpmPackageJsonFiles; + }; + time: { + modified: string; + created: string; + + [version: string]: string; + }; } export type NgAddSaveDepedency = 'dependencies' | 'devDependencies' | boolean; @@ -37,17 +55,16 @@ export interface PackageManifest { license?: string; private?: boolean; deprecated?: boolean; - - dependencies: PackageDependencies; - devDependencies: PackageDependencies; - peerDependencies: PackageDependencies; - optionalDependencies: PackageDependencies; + dependencies: Record; + devDependencies: Record; + peerDependencies: Record; + optionalDependencies: Record; 'ng-add'?: { save?: NgAddSaveDepedency; }; 'ng-update'?: { migrations: string; - packageGroup: { [name: string]: string }; + packageGroup: Record; }; } @@ -58,7 +75,9 @@ export interface PackageMetadata { 'dist-tags'?: unknown; } -type PackageManagerOptions = Record; +interface PackageManagerOptions extends Record { + forceAuth?: Record; +} let npmrc: PackageManagerOptions; @@ -122,47 +141,44 @@ 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); for (const [key, value] of Object.entries(rcConfig)) { + let substitutedValue = value; + + // Substitute any environment variable references. + if (typeof value === 'string') { + substitutedValue = value.replace(/\$\{([^\}]+)\}/, (_, name) => process.env[name] || ''); + } + switch (key) { case 'noproxy': case 'no-proxy': - options['noProxy'] = value; + options['noProxy'] = substitutedValue; break; case 'maxsockets': - options['maxSockets'] = value; + options['maxSockets'] = substitutedValue; break; case 'https-proxy': case 'proxy': - options['proxy'] = value; + options['proxy'] = substitutedValue; break; case 'strict-ssl': - options['strictSSL'] = value; + options['strictSSL'] = substitutedValue; break; case 'local-address': - options['localAddress'] = value; + options['localAddress'] = substitutedValue; break; case 'cafile': - if (typeof value === 'string') { - const cafile = path.resolve(path.dirname(location), value); + if (typeof substitutedValue === 'string') { + const cafile = path.resolve(path.dirname(location), substitutedValue); try { options['ca'] = readFileSync(cafile, 'utf8').replace(/\r?\n/g, '\n'); } catch {} } break; default: - options[key] = value; + options[key] = substitutedValue; break; } } - } else if (showPotentials) { - logger.info(`Trying '${location}'...not found.`); - } - } - - // Substitute any environment variable references - for (const key in options) { - const value = options[key]; - if (typeof value === 'string') { - options[key] = value.replace(/\$\{([^\}]+)\}/, (_, name) => process.env[name] || ''); } } @@ -238,18 +254,13 @@ export async function fetchPackageMetadata( export async function fetchPackageManifest( name: string, logger: logging.LoggerApi, - options?: { + options: { registry?: string; usingYarn?: boolean; verbose?: boolean; - }, + } = {}, ): Promise { - const { usingYarn, verbose, registry } = { - registry: undefined, - usingYarn: false, - verbose: false, - ...options, - }; + const { usingYarn = false, verbose = false, registry } = options; ensureNpmrc(logger, usingYarn, verbose); @@ -261,3 +272,49 @@ export async function fetchPackageManifest( return normalizeManifest(response); } + +export function getNpmPackageJson( + packageName: string, + logger: logging.LoggerApi, + options: { + registry?: string; + usingYarn?: boolean; + verbose?: boolean; + } = {}, +): Promise> { + const cachedResponse = npmPackageJsonCache.get(packageName); + if (cachedResponse) { + return cachedResponse; + } + + 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 {} + } + } + + const resultPromise: Promise = pacote.packument(packageName, { + fullMetadata: true, + ...npmrc, + ...(registry ? { registry } : {}), + }); + + // TODO: find some way to test this + const response = resultPromise.catch((err) => { + logger.warn(err.message || err); + + return { requestedName: packageName }; + }); + + npmPackageJsonCache.set(packageName, response); + + return response; +} From 5f4754b7f02e48073fb4d3a36c5982c787831efc Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Wed, 16 Jun 2021 13:05:40 +0200 Subject: [PATCH 2/3] fix(@angular/cli): handle unscoped authentication details in `.npmrc` files Unless auth options are scope with the registry url it appears that npm-registry-fetch ignores them, even though they are documented. https://github.com/npm/npm-registry-fetch/blob/8954f61d8d703e5eb7f3d93c9b40488f8b1b62ac/README.md https://github.com/npm/npm-registry-fetch/blob/8954f61d8d703e5eb7f3d93c9b40488f8b1b62ac/auth.js#L45-L91 --- .../cli/src/commands/update/schematic/index.ts | 5 ++++- packages/angular/cli/utilities/package-metadata.ts | 13 +++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/angular/cli/src/commands/update/schematic/index.ts b/packages/angular/cli/src/commands/update/schematic/index.ts index 528c501ddb3c..f3eab1c72f5d 100644 --- a/packages/angular/cli/src/commands/update/schematic/index.ts +++ b/packages/angular/cli/src/commands/update/schematic/index.ts @@ -11,7 +11,10 @@ import { Rule, SchematicContext, SchematicsException, Tree } from '@angular-devk import * as npa from 'npm-package-arg'; import * as semver from 'semver'; import { Dependency, JsonSchemaForNpmPackageJsonFiles } from '../../../../utilities/package-json'; -import { NpmRepositoryPackageJson, getNpmPackageJson } from '../../../../utilities/package-metadata'; +import { + NpmRepositoryPackageJson, + getNpmPackageJson, +} from '../../../../utilities/package-metadata'; import { Schema as UpdateSchema } from './schema'; type VersionRange = string & { __VERSION_RANGE: void }; diff --git a/packages/angular/cli/utilities/package-metadata.ts b/packages/angular/cli/utilities/package-metadata.ts index fe7a42b91254..b872848270a8 100644 --- a/packages/angular/cli/utilities/package-metadata.ts +++ b/packages/angular/cli/utilities/package-metadata.ts @@ -149,6 +149,19 @@ function readOptions( } switch (key) { + // Unless auth options are scope with the registry url it appears that npm-registry-fetch ignores them, + // even though they are documented. + // https://github.com/npm/npm-registry-fetch/blob/8954f61d8d703e5eb7f3d93c9b40488f8b1b62ac/README.md + // https://github.com/npm/npm-registry-fetch/blob/8954f61d8d703e5eb7f3d93c9b40488f8b1b62ac/auth.js#L45-L91 + case '_authToken': + case 'token': + case 'username': + case 'password': + case '_auth': + case 'auth': + options['forceAuth'] ??= {}; + options['forceAuth'][key] = substitutedValue; + break; case 'noproxy': case 'no-proxy': options['noProxy'] = substitutedValue; From f9f3618559e70334e576bee15086e880dd99d651 Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Wed, 16 Jun 2021 13:08:28 +0200 Subject: [PATCH 3/3] fix(@angular/cli): don't resolve `.npmrc` from parent directories Unlike `yarn`, `npm` doesn't resolve `.npmrc` from parent locations instead it looks in predefied locations in - per-project config file (/path/to/my/project/.npmrc) - per-user config file (~/.npmrc) - global config file ($PREFIX/etc/npmrc) - npm builtin config file (/path/to/npm/npmrc) https://docs.npmjs.com/cli/v7/configuring-npm/npmrc#files --- packages/angular/cli/utilities/package-metadata.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/angular/cli/utilities/package-metadata.ts b/packages/angular/cli/utilities/package-metadata.ts index b872848270a8..403428111d79 100644 --- a/packages/angular/cli/utilities/package-metadata.ts +++ b/packages/angular/cli/utilities/package-metadata.ts @@ -120,9 +120,11 @@ function readOptions( ]; const projectConfigLocations: string[] = [path.join(cwd, dotFilename)]; - const root = path.parse(cwd).root; - for (let curDir = path.dirname(cwd); curDir && curDir !== root; curDir = path.dirname(curDir)) { - projectConfigLocations.unshift(path.join(curDir, dotFilename)); + if (yarn) { + const root = path.parse(cwd).root; + for (let curDir = path.dirname(cwd); curDir && curDir !== root; curDir = path.dirname(curDir)) { + projectConfigLocations.unshift(path.join(curDir, dotFilename)); + } } if (showPotentials) {