Skip to content

Commit

Permalink
fix(@angular-devkit/schematics): filters out non-NPM dependencies
Browse files Browse the repository at this point in the history
Fixes angular#13059.

Non-NPM dependencies in package.json are now filtered out of `ng update`. Dependencies which are linked as tarball or Git references will now be skipped during the update process.
  • Loading branch information
dgp1130 committed Nov 18, 2019
1 parent bf2e58c commit dd9271c
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 18 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@
"@types/loader-utils": "^1.1.3",
"@types/minimist": "^1.2.0",
"@types/node": "10.12.30",
"@types/npm-package-arg": "^6.1.0",
"@types/request": "^2.47.1",
"@types/rimraf": "^2.0.2",
"@types/semver": "^6.0.0",
Expand Down
2 changes: 2 additions & 0 deletions packages/schematics/update/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ ts_library(
"//packages/angular_devkit/schematics",
"//packages/angular_devkit/schematics:tasks",
"@npm//@types/node",
"@npm//@types/npm-package-arg",
"@npm//@types/semver",
"@npm//npm-package-arg",
"@npm//rxjs",
"@npm//semver",
],
Expand Down
5 changes: 3 additions & 2 deletions packages/schematics/update/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@
"@angular-devkit/schematics": "0.0.0",
"@yarnpkg/lockfile": "1.1.0",
"ini": "1.3.5",
"npm-package-arg": "^7.0.0",
"pacote": "10.1.3",
"rxjs": "6.5.3",
"semver": "6.3.0",
"semver-intersect": "1.4.0",
"rxjs": "6.5.3"
"semver-intersect": "1.4.0"
}
}
46 changes: 33 additions & 13 deletions packages/schematics/update/update/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
Tree,
} from '@angular-devkit/schematics';
import { NodePackageInstallTask, RunSchematicTask } from '@angular-devkit/schematics/tasks';
import * as npa from 'npm-package-arg';
import { Observable, from as observableFrom, of } from 'rxjs';
import { map, mergeMap, reduce, switchMap } from 'rxjs/operators';
import * as semver from 'semver';
Expand Down Expand Up @@ -788,7 +789,7 @@ function _addPeerDependencies(
}


function _getAllDependencies(tree: Tree): Map<string, VersionRange> {
function _getAllDependencies(tree: Tree): Array<readonly [string, VersionRange]> {
const packageJsonContent = tree.read('/package.json');
if (!packageJsonContent) {
throw new SchematicsException('Could not find a package.json. Are you in a Node project?');
Expand All @@ -801,11 +802,11 @@ function _getAllDependencies(tree: Tree): Map<string, VersionRange> {
throw new SchematicsException('package.json could not be parsed: ' + e.message);
}

return new Map<string, VersionRange>([
...Object.entries(packageJson.peerDependencies || {}),
...Object.entries(packageJson.devDependencies || {}),
...Object.entries(packageJson.dependencies || {}),
] as [string, VersionRange][]);
return [
...Object.entries(packageJson.peerDependencies || {}) as Array<[string, VersionRange]>,
...Object.entries(packageJson.devDependencies || {}) as Array<[string, VersionRange]>,
...Object.entries(packageJson.dependencies || {}) as Array<[string, VersionRange]>,
];
}

function _formatVersion(version: string | undefined) {
Expand All @@ -826,6 +827,16 @@ function _formatVersion(version: string | undefined) {
return version;
}

/**
* Returns whether or not the given package spec (the value string in a
* `package.json` dependency) is hosted in the NPM registry.
* @throws When the spec cannot be parsed.
*/
function isPkgFromRegistry(name: string, spec: string): boolean {
const result = npa.resolve(name, spec);

return !!result.registry;
}

export default function(options: UpdateSchema): Rule {
if (!options.packages) {
Expand All @@ -847,14 +858,23 @@ export default function(options: UpdateSchema): Rule {

options.from = _formatVersion(options.from);
options.to = _formatVersion(options.to);
const usingYarn = options.packageManager === 'yarn';

return (tree: Tree, context: SchematicContext) => {
const logger = context.logger;
const allDependencies = _getAllDependencies(tree);
const packages = _buildPackageList(options, allDependencies, logger);
const usingYarn = options.packageManager === 'yarn';
const npmDeps = new Map(_getAllDependencies(tree).filter(([name, spec]) => {
try {
return isPkgFromRegistry(name, spec);
} catch {
// Abort on failure because package.json is malformed.
throw new SchematicsException(
`Failed to parse dependency "${name}" with spec "${spec}" from`
+ ` package.json. Is the spec malformed?`);
}
}));
const packages = _buildPackageList(options, npmDeps, logger);

return observableFrom([...allDependencies.keys()]).pipe(
return observableFrom(npmDeps.keys()).pipe(
// Grab all package.json from the npm repository. This requires a lot of HTTP calls so we
// try to parallelize as many as possible.
mergeMap(depName => getNpmPackageJson(
Expand Down Expand Up @@ -899,8 +919,8 @@ export default function(options: UpdateSchema): Rule {
do {
lastPackagesSize = packages.size;
npmPackageJsonMap.forEach((npmPackageJson) => {
_addPackageGroup(tree, packages, allDependencies, npmPackageJson, logger);
_addPeerDependencies(tree, packages, allDependencies, npmPackageJson, npmPackageJsonMap, logger);
_addPackageGroup(tree, packages, npmDeps, npmPackageJson, logger);
_addPeerDependencies(tree, packages, npmDeps, npmPackageJson, npmPackageJsonMap, logger);
});
} while (packages.size > lastPackagesSize);

Expand All @@ -909,7 +929,7 @@ export default function(options: UpdateSchema): Rule {
npmPackageJsonMap.forEach((npmPackageJson) => {
packageInfoMap.set(
npmPackageJson.name,
_buildPackageInfo(tree, packages, allDependencies, npmPackageJson, logger),
_buildPackageInfo(tree, packages, npmDeps, npmPackageJson, logger),
);
});

Expand Down
44 changes: 42 additions & 2 deletions packages/schematics/update/update/index_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
// tslint:disable:no-big-function

import { normalize, virtualFs } from '@angular-devkit/core';
import { HostTree } from '@angular-devkit/schematics';
import { HostTree, SchematicsException } from '@angular-devkit/schematics';
import { SchematicTestRunner, UnitTestTree } from '@angular-devkit/schematics/testing';
import { map } from 'rxjs/operators';
import { EMPTY } from 'rxjs';
import { catchError, map } from 'rxjs/operators';
import * as semver from 'semver';
import { angularMajorCompatGuarantee } from './index';

Expand Down Expand Up @@ -75,6 +76,45 @@ describe('@schematics/update', () => {
).toPromise().then(done, done.fail);
}, 45000);

it('ignores dependencies not hosted on the NPM registry', done => {
const tree = new UnitTestTree(new HostTree(new virtualFs.test.TestHost({
'/package.json': `{
"name": "blah",
"dependencies": {
"@angular-devkit-tests/update-base": "file:update-base-1.0.0.tgz"
}
}`,
})));

schematicRunner.runSchematicAsync('update', { all: true }, tree).pipe(
map(t => {
const packageJson = JSON.parse(t.readContent('/package.json'));
expect(packageJson['dependencies']['@angular-devkit-tests/update-base'])
.toBe('file:update-base-1.0.0.tgz');
}),
).toPromise().then(done, done.fail);
}, 45000);

it('emits SchematicsException when given an invalid dependency', done => {
const tree = new UnitTestTree(new HostTree(new virtualFs.test.TestHost({
'/package.json': `{
"name": "blah",
"dependencies": {
"//": "'abc://' is not a valid protocol for NPM.",
"@angular-devkit-tests/update-base": "abc://foo.tgz"
}
}`,
})));

schematicRunner.runSchematicAsync('update', { all: true }, tree).pipe(
catchError(err => {
expect(err instanceof SchematicsException).toBe(true);

return EMPTY;
}),
).toPromise().then(done, done.fail);
});

it('respects existing tilde and caret ranges', done => {
// Add ranges.
const content = virtualFs.fileBufferToString(host.sync.read(normalize('/package.json')));
Expand Down
6 changes: 5 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1166,6 +1166,11 @@
resolved "https://registry.yarnpkg.com/@types/normalize-package-data/-/normalize-package-data-2.4.0.tgz#e486d0d97396d79beedd0a6e33f4534ff6b4973e"
integrity sha512-f5j5b/Gf71L+dbqxIpQ4Z2WlmI/mPJ0fOkGGmFgtb6sAu97EPczzbS3/tJKxmcYDj55OX6ssqwDAWOHIYDRDGA==

"@types/npm-package-arg@^6.1.0":
version "6.1.0"
resolved "https://registry.yarnpkg.com/@types/npm-package-arg/-/npm-package-arg-6.1.0.tgz#88bdfce72f6a3d5fa1053c8d44d655e7850642e4"
integrity sha512-vbt5fb0y1svMhu++1lwtKmZL76d0uPChFlw7kEzyUmTwfmpHRcFb8i0R8ElT69q/L+QLgK2hgECivIAvaEDwag==

"@types/[email protected]", "@types/progress@^2.0.3":
version "2.0.3"
resolved "https://registry.yarnpkg.com/@types/progress/-/progress-2.0.3.tgz#7ccbd9c6d4d601319126c469e73b5bb90dfc8ccc"
Expand Down Expand Up @@ -9275,7 +9280,6 @@ sauce-connect-launcher@^1.2.4:

"sauce-connect-proxy@https://saucelabs.com/downloads/sc-4.5.4-linux.tar.gz":
version "0.0.0"
uid dc5efcd2be24ddb099a85b923d6e754754651fa8
resolved "https://saucelabs.com/downloads/sc-4.5.4-linux.tar.gz#dc5efcd2be24ddb099a85b923d6e754754651fa8"

saucelabs@^1.5.0:
Expand Down

0 comments on commit dd9271c

Please sign in to comment.