From 43f9d917d984c4aa8377216e064271bb18495c17 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Mon, 9 Oct 2017 13:11:13 +0300 Subject: [PATCH] build(aio): fix overwriting with local Angular packages that depend on other local ones (#19655) PR Close #19655 --- aio/tools/ng-packages-installer/index.js | 76 ++++++++++++++----- aio/tools/ng-packages-installer/index.spec.js | 72 ++++++++++++++++-- 2 files changed, 121 insertions(+), 27 deletions(-) diff --git a/aio/tools/ng-packages-installer/index.js b/aio/tools/ng-packages-installer/index.js index 786a804cc2..90785af76f 100644 --- a/aio/tools/ng-packages-installer/index.js +++ b/aio/tools/ng-packages-installer/index.js @@ -65,27 +65,61 @@ class NgPackagesInstaller { if (this._checkLocalMarker() !== true || this.force) { const pathToPackageConfig = path.resolve(this.projectDir, PACKAGE_JSON); const packages = this._getDistPackages(); - const packageConfigFile = fs.readFileSync(pathToPackageConfig); - const packageConfig = JSON.parse(packageConfigFile); - - 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); - - const localPackageConfig = Object.assign(Object.create(null), packageConfig, { dependencies, devDependencies }); - localPackageConfig.__angular = { local: true }; - const localPackageConfigJson = JSON.stringify(localPackageConfig, null, 2); try { - this._log(`Writing temporary local ${PACKAGE_JSON} to ${pathToPackageConfig}`); - fs.writeFileSync(pathToPackageConfig, localPackageConfigJson); - this._installDeps('--no-lockfile', '--check-files'); - this._setLocalMarker(localPackageConfigJson); + // Overwrite local Angular packages dependencies to other Angular packages with local files. + Object.keys(packages).forEach(key => { + const pkg = packages[key]; + const tmpConfig = JSON.parse(JSON.stringify(pkg.config)); + + // Prevent accidental publishing of the package, if something goes wrong. + tmpConfig.private = true; + + // Overwrite project dependencies/devDependencies to Angular packages with local files. + ['dependencies', 'devDependencies'].forEach(prop => { + const deps = tmpConfig[prop] || {}; + Object.keys(deps).forEach(key2 => { + const pkg2 = packages[key2]; + if (pkg2) { + // point the core Angular packages at the distributable folder + deps[key2] = `file:${pkg2.parentDir}/${key2.replace('@angular/', '')}`; + this._log(`Overriding dependency of local ${key} with local package: ${key2}: ${deps[key2]}`); + } + }); + }); + + fs.writeFileSync(pkg.packageJsonPath, JSON.stringify(tmpConfig)); + }); + + const packageConfigFile = fs.readFileSync(pathToPackageConfig); + const packageConfig = JSON.parse(packageConfigFile); + + 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); + + const localPackageConfig = Object.assign(Object.create(null), packageConfig, { dependencies, devDependencies }); + localPackageConfig.__angular = { local: true }; + const localPackageConfigJson = JSON.stringify(localPackageConfig, null, 2); + + try { + this._log(`Writing temporary local ${PACKAGE_JSON} to ${pathToPackageConfig}`); + fs.writeFileSync(pathToPackageConfig, localPackageConfigJson); + this._installDeps('--no-lockfile', '--check-files'); + this._setLocalMarker(localPackageConfigJson); + } finally { + this._log(`Restoring original ${PACKAGE_JSON} to ${pathToPackageConfig}`); + fs.writeFileSync(pathToPackageConfig, packageConfigFile); + } } finally { - this._log(`Restoring original ${PACKAGE_JSON} to ${pathToPackageConfig}`); - fs.writeFileSync(pathToPackageConfig, packageConfigFile); + // Restore local Angular packages dependencies to other Angular packages. + this._log(`Restoring original ${PACKAGE_JSON} for local Angular packages.`); + Object.keys(packages).forEach(key => { + const pkg = packages[key]; + fs.writeFileSync(pkg.packageJsonPath, JSON.stringify(pkg.config)); + }); } } } @@ -151,7 +185,11 @@ class NgPackagesInstaller { const packageName = `@angular/${packagePath.slice(0, -PACKAGE_JSON.length -1)}`; if (this.ignorePackages.indexOf(packageName) === -1) { const packageConfig = require(path.resolve(distDir, packagePath)); - packageConfigs[packageName] = {parentDir: distDir, config: packageConfig}; + packageConfigs[packageName] = { + parentDir: distDir, + packageJsonPath: path.resolve(distDir, packagePath), + config: packageConfig + }; } else { this._log('Ignoring package', packageName); } diff --git a/aio/tools/ng-packages-installer/index.spec.js b/aio/tools/ng-packages-installer/index.spec.js index 0cc46033c9..641bff294f 100644 --- a/aio/tools/ng-packages-installer/index.spec.js +++ b/aio/tools/ng-packages-installer/index.spec.js @@ -47,6 +47,7 @@ describe('NgPackagesInstaller', () => { }); describe('installLocalDependencies()', () => { + const copyJsonObj = obj => JSON.parse(JSON.stringify(obj)); let dummyNgPackages, dummyPackage, dummyPackageJson, expectedModifiedPackage, expectedModifiedPackageJson; beforeEach(() => { @@ -54,12 +55,39 @@ describe('NgPackagesInstaller', () => { // These are the packages that are "found" in the dist directory dummyNgPackages = { - '@angular/core': { parentDir: packagesDir, config: { peerDependencies: { rxjs: '5.0.1' } } }, - '@angular/common': { parentDir: packagesDir, config: { peerDependencies: { '@angular/core': '4.4.1' } } }, - '@angular/compiler': { parentDir: packagesDir, config: { } }, - '@angular/compiler-cli': { parentDir: toolsDir, config: { peerDependencies: { typescript: '^2.4.2', '@angular/compiler': '4.3.2' } } } + '@angular/core': { + parentDir: packagesDir, + packageJsonPath: `${packagesDir}/core/package.json`, + config: { peerDependencies: { rxjs: '5.0.1' } } + }, + '@angular/common': { + parentDir: packagesDir, + packageJsonPath: `${packagesDir}/common/package.json`, + config: { peerDependencies: { '@angular/core': '4.4.4-1ab23cd4' } } + }, + '@angular/compiler': { + parentDir: packagesDir, + packageJsonPath: `${packagesDir}/compiler/package.json`, + config: { peerDependencies: { '@angular/common': '4.4.4-1ab23cd4' } } + }, + '@angular/compiler-cli': { + parentDir: toolsDir, + packageJsonPath: `${toolsDir}/compiler-cli/package.json`, + config: { + dependencies: { '@angular/tsc-wrapped': '4.4.4-1ab23cd4' }, + peerDependencies: { typescript: '^2.4.2', '@angular/compiler': '4.4.4-1ab23cd4' } + } + }, + '@angular/tsc-wrapped': { + parentDir: toolsDir, + packageJsonPath: `${toolsDir}/tsc-wrapped/package.json`, + config: { + devDependencies: { '@angular/common': '4.4.4-1ab23cd4' }, + peerDependencies: { tsickle: '^1.4.0' } + } + } }; - spyOn(installer, '_getDistPackages').and.returnValue(dummyNgPackages); + spyOn(installer, '_getDistPackages').and.callFake(() => copyJsonObj(dummyNgPackages)); // This is the package.json in the "test" folder dummyPackage = { @@ -118,6 +146,33 @@ describe('NgPackagesInstaller', () => { expect(installer._getDistPackages).toHaveBeenCalled(); }); + it('should temporarily overwrite the package.json files of local Angular packages', () => { + const pkgJsonFor = pkgName => dummyNgPackages[`@angular/${pkgName}`].packageJsonPath; + const pkgConfigFor = pkgName => copyJsonObj(dummyNgPackages[`@angular/${pkgName}`].config); + const overwriteConfigFor = (pkgName, newProps) => Object.assign(pkgConfigFor(pkgName), newProps); + + const allArgs = fs.writeFileSync.calls.allArgs(); + const firstFiveArgs = allArgs.slice(0, 5); + const lastFiveArgs = allArgs.slice(-5); + + expect(firstFiveArgs).toEqual([ + [pkgJsonFor('core'), JSON.stringify(overwriteConfigFor('core', {private: true}))], + [pkgJsonFor('common'), JSON.stringify(overwriteConfigFor('common', {private: true}))], + [pkgJsonFor('compiler'), JSON.stringify(overwriteConfigFor('compiler', {private: true}))], + [pkgJsonFor('compiler-cli'), JSON.stringify(overwriteConfigFor('compiler-cli', { + private: true, + dependencies: { '@angular/tsc-wrapped': `file:${toolsDir}/tsc-wrapped` } + }))], + [pkgJsonFor('tsc-wrapped'), JSON.stringify(overwriteConfigFor('tsc-wrapped', { + private: true, + devDependencies: { '@angular/common': `file:${packagesDir}/common` } + }))], + ]); + + expect(lastFiveArgs).toEqual(['core', 'common', 'compiler', 'compiler-cli', 'tsc-wrapped'] + .map(pkgName => [pkgJsonFor(pkgName), JSON.stringify(pkgConfigFor(pkgName))])); + }); + it('should load the package.json', () => { expect(fs.readFileSync).toHaveBeenCalledWith(packageJsonPath); }); @@ -152,11 +207,12 @@ describe('NgPackagesInstaller', () => { }); }); - describe('_getDistPackages', () => { + describe('_getDistPackages()', () => { it('should include top level Angular packages', () => { const ngPackages = installer._getDistPackages(); const expectedValue = jasmine.objectContaining({ parentDir: jasmine.any(String), + packageJsonPath: jasmine.any(String), config: jasmine.any(Object), }); @@ -208,7 +264,7 @@ describe('NgPackagesInstaller', () => { }); }); - describe('_printWarning', () => { + describe('_printWarning()', () => { it('should mention the message passed in the warning', () => { installer._printWarning(); expect(console.warn.calls.argsFor(0)[0]).toContain('is running against the local Angular build'); @@ -231,7 +287,7 @@ describe('NgPackagesInstaller', () => { }); }); - describe('_installDeps', () => { + describe('_installDeps()', () => { it('should run yarn install with the given options', () => { installer._installDeps('option-1', 'option-2'); expect(shelljs.exec).toHaveBeenCalledWith('yarn install option-1 option-2', { cwd: absoluteRootDir });