build(docs-infra): use pinned dependencies when possible in `ng-packages-installer` (#28510)
Previously, `ng-packages-installer` would replace the version ranges for all dependencies that were peer dependencies of an Angular package with the version range used in the Angular package. This effectively meant that the pinned version (from `yarn.lock`) for that dependency was ignored (even if the pinned version satisfied the new version range). This commit reduces non-determinism in CI jobs using the locally built Angular packages by always using pinned versions of dependencies for Angular package peer dependencies if possible. For example, assuming the following versions for the RxJS dependency: - **aio/package.json**: `rxjs: ^6.3.0` - **aio/yarn.lock**: `rxjs@^6.3.0: 6.3.3` - **@angular/core#peerDependencies**: `rxjs: ^6.0.0` ...the following versions would be used with `ng-packages-installer`: - Before this commit: - **aio/package.json**: `rxjs: ^6.0.0` - **node_modules/rxjs/**: `6.4.0` (latest version satisfying `^6.0.0`) - After this commit: - **aio/package.json**: `rxjs: ^6.3.0` - **node_modules/rxjs/**: `6.3.3` (because it satisfies `^6.0.0`) PR Close #28510
This commit is contained in:
George Kalpakas
Misko Hevery
parent
9ce0c23c77
commit
3de06dd794
|
|
@ -104,6 +104,7 @@
|
|||
"@types/jasmine": "^2.5.52",
|
||||
"@types/jasminewd2": "^2.0.4",
|
||||
"@types/node": "~6.0.60",
|
||||
"@yarnpkg/lockfile": "^1.1.0",
|
||||
"archiver": "^1.3.0",
|
||||
"canonical-path": "1.0.0",
|
||||
"chalk": "^2.1.0",
|
||||
|
|
|
|||
|
|
@ -2,11 +2,14 @@
|
|||
|
||||
const chalk = require('chalk');
|
||||
const fs = require('fs-extra');
|
||||
const lockfile = require('@yarnpkg/lockfile');
|
||||
const path = require('canonical-path');
|
||||
const semver = require('semver');
|
||||
const shelljs = require('shelljs');
|
||||
const yargs = require('yargs');
|
||||
|
||||
const PACKAGE_JSON = 'package.json';
|
||||
const YARN_LOCK = 'yarn.lock';
|
||||
const LOCAL_MARKER_PATH = 'node_modules/_local_.json';
|
||||
const PACKAGE_JSON_REGEX = /^[^/]+\/package\.json$/;
|
||||
|
||||
|
|
@ -64,6 +67,8 @@ class NgPackagesInstaller {
|
|||
installLocalDependencies() {
|
||||
if (this.force || !this._checkLocalMarker()) {
|
||||
const pathToPackageConfig = path.resolve(this.projectDir, PACKAGE_JSON);
|
||||
const pathToLockfile = path.resolve(this.projectDir, YARN_LOCK);
|
||||
const parsedLockfile = this._parseLockfile(pathToLockfile);
|
||||
const packages = this._getDistPackages();
|
||||
|
||||
try {
|
||||
|
|
@ -97,8 +102,8 @@ class NgPackagesInstaller {
|
|||
const [dependencies, peers] = this._collectDependencies(packageConfig.dependencies || {}, packages);
|
||||
const [devDependencies, devPeers] = this._collectDependencies(packageConfig.devDependencies || {}, packages);
|
||||
|
||||
this._assignPeerDependencies(peers, dependencies, devDependencies);
|
||||
this._assignPeerDependencies(devPeers, dependencies, devDependencies);
|
||||
this._assignPeerDependencies(peers, dependencies, devDependencies, parsedLockfile);
|
||||
this._assignPeerDependencies(devPeers, dependencies, devDependencies, parsedLockfile);
|
||||
|
||||
const localPackageConfig = Object.assign(Object.create(null), packageConfig, { dependencies, devDependencies });
|
||||
localPackageConfig.__angular = { local: true };
|
||||
|
|
@ -134,15 +139,23 @@ class NgPackagesInstaller {
|
|||
|
||||
// Protected helpers
|
||||
|
||||
_assignPeerDependencies(peerDependencies, dependencies, devDependencies) {
|
||||
_assignPeerDependencies(peerDependencies, dependencies, devDependencies, parsedLockfile) {
|
||||
Object.keys(peerDependencies).forEach(key => {
|
||||
const peerDepRange = peerDependencies[key];
|
||||
|
||||
// Ignore peerDependencies whose range is already satisfied by current version in lockfile.
|
||||
const originalRange = dependencies[key] || devDependencies[key];
|
||||
const lockfileVersion = originalRange && parsedLockfile[`${key}@${originalRange}`].version;
|
||||
|
||||
if (lockfileVersion && semver.satisfies(lockfileVersion, peerDepRange)) return;
|
||||
|
||||
// If there is already an equivalent dependency then override it - otherwise assign/override the devDependency
|
||||
if (dependencies[key]) {
|
||||
this._log(`Overriding dependency with peerDependency: ${key}: ${peerDependencies[key]}`);
|
||||
dependencies[key] = peerDependencies[key];
|
||||
this._log(`Overriding dependency with peerDependency: ${key}: ${peerDepRange}`);
|
||||
dependencies[key] = peerDepRange;
|
||||
} else {
|
||||
this._log(`${devDependencies[key] ? 'Overriding' : 'Assigning'} devDependency with peerDependency: ${key}: ${peerDependencies[key]}`);
|
||||
devDependencies[key] = peerDependencies[key];
|
||||
this._log(`${devDependencies[key] ? 'Overriding' : 'Assigning'} devDependency with peerDependency: ${key}: ${peerDepRange}`);
|
||||
devDependencies[key] = peerDepRange;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
|
@ -221,6 +234,20 @@ class NgPackagesInstaller {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse and return a `yarn.lock` file.
|
||||
*/
|
||||
_parseLockfile(lockfilePath) {
|
||||
const lockfileContent = fs.readFileSync(lockfilePath, 'utf8');
|
||||
const parsed = lockfile.parse(lockfileContent);
|
||||
|
||||
if (parsed.type !== 'success') {
|
||||
throw new Error(`[${NgPackagesInstaller.name}]: Error parsing lockfile '${lockfilePath}' (result type: ${parsed.type}).`);
|
||||
}
|
||||
|
||||
return parsed.object;
|
||||
}
|
||||
|
||||
_printWarning() {
|
||||
const relativeScriptPath = path.relative('.', __filename.replace(/\.js$/, ''));
|
||||
const absoluteProjectDir = path.resolve(this.projectDir);
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
'use strict';
|
||||
|
||||
const fs = require('fs-extra');
|
||||
const lockfile = require('@yarnpkg/lockfile');
|
||||
const path = require('canonical-path');
|
||||
const shelljs = require('shelljs');
|
||||
|
||||
|
|
@ -11,6 +12,7 @@ describe('NgPackagesInstaller', () => {
|
|||
const absoluteRootDir = path.resolve(rootDir);
|
||||
const nodeModulesDir = path.resolve(absoluteRootDir, 'node_modules');
|
||||
const packageJsonPath = path.resolve(absoluteRootDir, 'package.json');
|
||||
const yarnLockPath = path.resolve(absoluteRootDir, 'yarn.lock');
|
||||
const packagesDir = path.resolve(path.resolve(__dirname, '../../../dist/packages-dist'));
|
||||
const toolsDir = path.resolve(path.resolve(__dirname, '../../../dist/tools/@angular'));
|
||||
let installer;
|
||||
|
|
@ -55,12 +57,23 @@ describe('NgPackagesInstaller', () => {
|
|||
spyOn(installer, '_installDeps');
|
||||
spyOn(installer, '_setLocalMarker');
|
||||
|
||||
spyOn(installer, '_parseLockfile').and.returnValue({
|
||||
'rxjs@^6.3.0': {version: '6.3.3'},
|
||||
'zone.js@^0.8.26': {version: '0.8.27'}
|
||||
});
|
||||
|
||||
// These are the packages that are "found" in the dist directory
|
||||
dummyNgPackages = {
|
||||
'@angular/core': {
|
||||
parentDir: packagesDir,
|
||||
packageJsonPath: `${packagesDir}/core/package.json`,
|
||||
config: { peerDependencies: { 'some-package': '5.0.1' } }
|
||||
config: {
|
||||
peerDependencies: {
|
||||
'rxjs': '^6.4.0',
|
||||
'some-package': '5.0.1',
|
||||
'zone.js': '~0.8.26'
|
||||
}
|
||||
}
|
||||
},
|
||||
'@angular/common': {
|
||||
parentDir: packagesDir,
|
||||
|
|
@ -95,10 +108,12 @@ describe('NgPackagesInstaller', () => {
|
|||
dummyPackage = {
|
||||
dependencies: {
|
||||
'@angular/core': '4.4.1',
|
||||
'@angular/common': '4.4.1'
|
||||
'@angular/common': '4.4.1',
|
||||
rxjs: '^6.3.0'
|
||||
},
|
||||
devDependencies: {
|
||||
'@angular/compiler-cli': '4.4.1'
|
||||
'@angular/compiler-cli': '4.4.1',
|
||||
'zone.js': '^0.8.26'
|
||||
}
|
||||
};
|
||||
dummyPackageJson = JSON.stringify(dummyPackage);
|
||||
|
|
@ -106,14 +121,23 @@ describe('NgPackagesInstaller', () => {
|
|||
|
||||
// This is the package.json that is temporarily written to the "test" folder
|
||||
// Note that the Angular (dev)dependencies have been modified to use a "file:" path
|
||||
// And that the peerDependencies from `dummyNgPackages` have been added as (dev)dependencies.
|
||||
// And that the peerDependencies from `dummyNgPackages` have been updated or added as
|
||||
// (dev)dependencies (unless the current version in lockfile satisfies semver).
|
||||
//
|
||||
// For example, `zone.js@0.8.27` (from lockfile) satisfies `zone.js@~0.8.26` (from
|
||||
// `@angular/core`), thus `zone.js: ^0.8.26` (from original `package.json`) is retained.
|
||||
// In contrast, `rxjs@6.3.3` (from lockfile) does not satisfy `rxjs@^6.4.0 (from
|
||||
// `@angular/core`), thus `rxjs: ^6.3.0` (from original `package.json`) is replaced with
|
||||
// `rxjs: ^6.4.0` (from `@angular/core`).
|
||||
expectedModifiedPackage = {
|
||||
dependencies: {
|
||||
'@angular/core': `file:${packagesDir}/core`,
|
||||
'@angular/common': `file:${packagesDir}/common`
|
||||
'@angular/common': `file:${packagesDir}/common`,
|
||||
'rxjs': '^6.4.0'
|
||||
},
|
||||
devDependencies: {
|
||||
'@angular/compiler-cli': `file:${toolsDir}/compiler-cli`,
|
||||
'zone.js': '^0.8.26',
|
||||
'some-package': '5.0.1',
|
||||
typescript: '^2.4.2'
|
||||
},
|
||||
|
|
@ -150,8 +174,9 @@ describe('NgPackagesInstaller', () => {
|
|||
installer.installLocalDependencies();
|
||||
});
|
||||
|
||||
it('should get the dist packages', () => {
|
||||
it('should parse the lockfile and get the dist packages', () => {
|
||||
expect(installer._checkLocalMarker).toHaveBeenCalled();
|
||||
expect(installer._parseLockfile).toHaveBeenCalledWith(yarnLockPath);
|
||||
expect(installer._getDistPackages).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
|
|
@ -274,6 +299,42 @@ describe('NgPackagesInstaller', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('_parseLockfile()', () => {
|
||||
let originalLockfileParseDescriptor;
|
||||
|
||||
beforeEach(() => {
|
||||
// Workaround for `lockfile.parse()` being non-writable.
|
||||
let parse = lockfile.parse;
|
||||
originalLockfileParseDescriptor = Object.getOwnPropertyDescriptor(lockfile, 'parse');
|
||||
Object.defineProperty(lockfile, 'parse', {
|
||||
get() { return parse; },
|
||||
set(newParse) { parse = newParse; },
|
||||
});
|
||||
|
||||
fs.readFileSync.and.returnValue('mock content');
|
||||
spyOn(lockfile, 'parse').and.returnValue({type: 'success', object: {foo: {version: 'bar'}}});
|
||||
});
|
||||
|
||||
afterEach(() => Object.defineProperty(lockfile, 'parse', originalLockfileParseDescriptor));
|
||||
|
||||
it('should parse the specified lockfile', () => {
|
||||
installer._parseLockfile('/foo/bar/yarn.lock');
|
||||
expect(fs.readFileSync).toHaveBeenCalledWith('/foo/bar/yarn.lock', 'utf8');
|
||||
expect(lockfile.parse).toHaveBeenCalledWith('mock content');
|
||||
});
|
||||
|
||||
it('should throw if parsing the lockfile fails', () => {
|
||||
lockfile.parse.and.returnValue({type: 'not success'});
|
||||
expect(() => installer._parseLockfile('/foo/bar/yarn.lock')).toThrowError(
|
||||
'[NgPackagesInstaller]: Error parsing lockfile \'/foo/bar/yarn.lock\' (result type: not success).');
|
||||
});
|
||||
|
||||
it('should return the parsed lockfile content as an object', () => {
|
||||
const parsed = installer._parseLockfile('/foo/bar/yarn.lock');
|
||||
expect(parsed).toEqual({foo: {version: 'bar'}});
|
||||
});
|
||||
});
|
||||
|
||||
describe('_printWarning()', () => {
|
||||
it('should mention the message passed in the warning', () => {
|
||||
installer._printWarning();
|
||||
|
|
|
|||
|
|
@ -537,7 +537,7 @@
|
|||
version "4.2.1"
|
||||
resolved "https://registry.yarnpkg.com/@xtuc/long/-/long-4.2.1.tgz#5c85d662f76fa1d34575766c5dcd6615abcd30d8"
|
||||
|
||||
"@yarnpkg/lockfile@1.1.0":
|
||||
"@yarnpkg/lockfile@1.1.0", "@yarnpkg/lockfile@^1.1.0":
|
||||
version "1.1.0"
|
||||
resolved "https://registry.yarnpkg.com/@yarnpkg/lockfile/-/lockfile-1.1.0.tgz#e77a97fbd345b76d83245edcd17d393b1b41fb31"
|
||||
integrity sha512-GpSwvyXOcOOlV70vbnzjj4fW5xW/FdUF6nQEt1ENy7m4ZCczi1+/buVUPAqmGfqznsORNFzUMjctTIp8a9tuCQ==
|
||||
|
|
|
|||
Loading…
Reference in New Issue