diff --git a/aio/package.json b/aio/package.json index c681203634..94e2234b38 100644 --- a/aio/package.json +++ b/aio/package.json @@ -97,6 +97,7 @@ "@types/jasmine": "^2.5.52", "@types/jasminewd2": "^2.0.3", "@types/node": "~6.0.60", + "@yarnpkg/lockfile": "^1.1.0", "archiver": "^1.3.0", "canonical-path": "^0.0.2", "chalk": "^2.1.0", diff --git a/aio/tools/ng-packages-installer/index.js b/aio/tools/ng-packages-installer/index.js index 85cd65f3d2..cef6fa79e2 100644 --- a/aio/tools/ng-packages-installer/index.js +++ b/aio/tools/ng-packages-installer/index.js @@ -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; } }); } @@ -220,6 +233,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); diff --git a/aio/tools/ng-packages-installer/index.spec.js b/aio/tools/ng-packages-installer/index.spec.js index 7c6cbb95a8..681abd1d9f 100644 --- a/aio/tools/ng-packages-installer/index.spec.js +++ b/aio/tools/ng-packages-installer/index.spec.js @@ -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: { rxjs: '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,15 +121,24 @@ 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`, - rxjs: '5.0.1', + 'zone.js': '^0.8.26', + 'some-package': '5.0.1', typescript: '^2.4.2' }, __angular: { local: true } @@ -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(); diff --git a/aio/yarn.lock b/aio/yarn.lock index 31d89eaa3c..b36f8d8be8 100644 --- a/aio/yarn.lock +++ b/aio/yarn.lock @@ -338,6 +338,10 @@ dependencies: "@types/node" "*" +"@yarnpkg/lockfile@^1.1.0": + version "1.1.0" + resolved "https://registry.yarnpkg.com/@yarnpkg/lockfile/-/lockfile-1.1.0.tgz#e77a97fbd345b76d83245edcd17d393b1b41fb31" + JSONStream@^1.2.1: version "1.3.1" resolved "https://registry.yarnpkg.com/JSONStream/-/JSONStream-1.3.1.tgz#707f761e01dae9e16f1bcf93703b78c70966579a"