refactor(docs-infra): simplify NgPackagesInstaller and speed up local package detection (#35780)
				
					
				
			Previously, `NgPackagesInstaller` would only look for Angular local packages and do so by listing all (deeply nested) files in `dist/packages-dist/` and looking for `package.json` files nested two levels deep (i.e. `dist/packages-dist/*/package.json`). Thus, it would unnecessarily check a large number of files. This commit changes the package detection logic to instead look for a `package.json` file directly inside each subdirectory of `dist/packages-dist/`, which speeds up the operation. It also refactors the code to make it easier to look for packages in other directories (besides `dist/packages-dist/`). This will be useful in a subsequent commit to look for and use the locally built `zone.js` package (from `dist/zone.js-dist/`). PR Close #35780
This commit is contained in:
		
							parent
							
								
									3f88de9407
								
							
						
					
					
						commit
						4c7d833233
					
				| @ -11,12 +11,11 @@ const yargs = require('yargs'); | |||||||
| const PACKAGE_JSON = 'package.json'; | const PACKAGE_JSON = 'package.json'; | ||||||
| const YARN_LOCK = 'yarn.lock'; | const YARN_LOCK = 'yarn.lock'; | ||||||
| const LOCAL_MARKER_PATH = 'node_modules/_local_.json'; | const LOCAL_MARKER_PATH = 'node_modules/_local_.json'; | ||||||
| const PACKAGE_JSON_REGEX = /^[^/]+\/package\.json$/; |  | ||||||
| 
 | 
 | ||||||
| const ANGULAR_ROOT_DIR = path.resolve(__dirname, '../../..'); | const ANGULAR_ROOT_DIR = path.resolve(__dirname, '../../..'); | ||||||
| const ANGULAR_DIST_PACKAGES = path.join(ANGULAR_ROOT_DIR, 'dist/packages-dist'); | const ANGULAR_DIST_PACKAGES_DIR = path.join(ANGULAR_ROOT_DIR, 'dist/packages-dist'); | ||||||
| const ANGULAR_DIST_PACKAGES_BUILD_SCRIPT = path.join(ANGULAR_ROOT_DIR, 'scripts/build/build-packages-dist.js'); | const DIST_PACKAGES_BUILD_SCRIPT = path.join(ANGULAR_ROOT_DIR, 'scripts/build/build-packages-dist.js'); | ||||||
| const ANGULAR_DIST_PACKAGES_BUILD_CMD = `"${process.execPath}" "${ANGULAR_DIST_PACKAGES_BUILD_SCRIPT}"`; | const DIST_PACKAGES_BUILD_CMD = `"${process.execPath}" "${DIST_PACKAGES_BUILD_SCRIPT}"`; | ||||||
| 
 | 
 | ||||||
| /** | /** | ||||||
|  * A tool that can install Angular dependencies for a project from NPM or from the |  * A tool that can install Angular dependencies for a project from NPM or from the | ||||||
| @ -94,7 +93,7 @@ class NgPackagesInstaller { | |||||||
|               const pkg2 = packages[key2]; |               const pkg2 = packages[key2]; | ||||||
|               if (pkg2) { |               if (pkg2) { | ||||||
|                 // point the core Angular packages at the distributable folder
 |                 // point the core Angular packages at the distributable folder
 | ||||||
|                 deps[key2] = `file:${pkg2.parentDir}/${key2.replace('@angular/', '')}`; |                 deps[key2] = `file:${pkg2.packageDir}`; | ||||||
|                 this._log(`Overriding dependency of local ${key} with local package: ${key2}: ${deps[key2]}`); |                 this._log(`Overriding dependency of local ${key} with local package: ${key2}: ${deps[key2]}`); | ||||||
|               } |               } | ||||||
|             }); |             }); | ||||||
| @ -178,14 +177,14 @@ class NgPackagesInstaller { | |||||||
|     const canBuild = process.platform !== 'win32'; |     const canBuild = process.platform !== 'win32'; | ||||||
| 
 | 
 | ||||||
|     if (canBuild) { |     if (canBuild) { | ||||||
|       this._log(`Building the Angular packages with: ${ANGULAR_DIST_PACKAGES_BUILD_SCRIPT}`); |       this._log(`Building the Angular packages with: ${DIST_PACKAGES_BUILD_SCRIPT}`); | ||||||
|       shelljs.exec(ANGULAR_DIST_PACKAGES_BUILD_CMD); |       shelljs.exec(DIST_PACKAGES_BUILD_CMD); | ||||||
|     } else { |     } else { | ||||||
|       this._warn([ |       this._warn([ | ||||||
|         'Automatically building the local Angular packages is currently not supported on Windows.', |         'Automatically building the local Angular packages is currently not supported on Windows.', | ||||||
|         `Please, ensure '${ANGULAR_DIST_PACKAGES}' exists and is up-to-date (e.g. by running ` + |         `Please, ensure '${ANGULAR_DIST_PACKAGES_DIR}' exists and is up-to-date (e.g. by running ` + | ||||||
|           `'${ANGULAR_DIST_PACKAGES_BUILD_SCRIPT}' in Git Bash for Windows, Windows Subsystem for Linux or a Linux ` + |           `'${DIST_PACKAGES_BUILD_SCRIPT}' in Git Bash for Windows, Windows Subsystem for Linux or a Linux docker ` + | ||||||
|           'docker container or VM).', |           'container or VM).', | ||||||
|         '', |         '', | ||||||
|         'Proceeding anyway...', |         'Proceeding anyway...', | ||||||
|       ].join('\n')); |       ].join('\n')); | ||||||
| @ -200,7 +199,7 @@ class NgPackagesInstaller { | |||||||
|       const sourcePackage = packages[key]; |       const sourcePackage = packages[key]; | ||||||
|       if (sourcePackage) { |       if (sourcePackage) { | ||||||
|         // point the core Angular packages at the distributable folder
 |         // point the core Angular packages at the distributable folder
 | ||||||
|         mergedDependencies[key] = `file:${sourcePackage.parentDir}/${key.replace('@angular/', '')}`; |         mergedDependencies[key] = `file:${sourcePackage.packageDir}`; | ||||||
|         this._log(`Overriding dependency with local package: ${key}: ${mergedDependencies[key]}`); |         this._log(`Overriding dependency with local package: ${key}: ${mergedDependencies[key]}`); | ||||||
|         // grab peer dependencies
 |         // grab peer dependencies
 | ||||||
|         const sourcePackagePeerDeps = sourcePackage.config.peerDependencies || {}; |         const sourcePackagePeerDeps = sourcePackage.config.peerDependencies || {}; | ||||||
| @ -219,32 +218,43 @@ class NgPackagesInstaller { | |||||||
|    * (Detected as directories in '/dist/packages-dist/' that contain a top-level 'package.json' file.) |    * (Detected as directories in '/dist/packages-dist/' that contain a top-level 'package.json' file.) | ||||||
|    */ |    */ | ||||||
|   _getDistPackages() { |   _getDistPackages() { | ||||||
|     const packageConfigs = Object.create(null); |     this._log(`Angular distributable directory: ${ANGULAR_DIST_PACKAGES_DIR}.`); | ||||||
|     const distDir = ANGULAR_DIST_PACKAGES; |  | ||||||
| 
 |  | ||||||
|     this._log(`Angular distributable directory: ${distDir}.`); |  | ||||||
| 
 | 
 | ||||||
|     if (this.buildPackages) { |     if (this.buildPackages) { | ||||||
|       this._buildDistPackages(); |       this._buildDistPackages(); | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     shelljs |     const collectPackages = containingDir => { | ||||||
|       .find(distDir) |       const packages = {}; | ||||||
|       .map(filePath => filePath.slice(distDir.length + 1)) | 
 | ||||||
|       .filter(filePath => PACKAGE_JSON_REGEX.test(filePath)) |       for (const dirName of shelljs.ls(containingDir)) { | ||||||
|       .forEach(packagePath => { |         const packageDir = path.resolve(containingDir, dirName); | ||||||
|         const packageName = `@angular/${packagePath.slice(0, -PACKAGE_JSON.length -1)}`; |         const packageJsonPath = path.join(packageDir, PACKAGE_JSON); | ||||||
|         if (this.ignorePackages.indexOf(packageName) === -1) { |         const packageConfig = fs.existsSync(packageJsonPath) ? require(packageJsonPath) : null; | ||||||
|           const packageConfig = require(path.resolve(distDir, packagePath)); |         const packageName = packageConfig && packageConfig.name; | ||||||
|           packageConfigs[packageName] = { | 
 | ||||||
|             parentDir: distDir, |         if (!packageConfig) { | ||||||
|             packageJsonPath: path.resolve(distDir, packagePath), |           // No `package.json` found - this directory is not a package.
 | ||||||
|             config: packageConfig |           continue; | ||||||
|           }; |         } else if (!packageName) { | ||||||
|         } else { |           // No `name` property in `package.json`. (This should never happen.)
 | ||||||
|           this._log('Ignoring package', packageName); |           throw new Error(`Package '${packageDir}' specifies no name in its '${PACKAGE_JSON}'.`); | ||||||
|  |         } else if (this.ignorePackages.includes(packageName)) { | ||||||
|  |           this._log(`Ignoring package '${packageName}'.`); | ||||||
|  |           continue; | ||||||
|         } |         } | ||||||
|       }); | 
 | ||||||
|  |         packages[packageName] = { | ||||||
|  |           packageDir, | ||||||
|  |           packageJsonPath, | ||||||
|  |           config: packageConfig, | ||||||
|  |         }; | ||||||
|  |       } | ||||||
|  | 
 | ||||||
|  |       return packages; | ||||||
|  |     }; | ||||||
|  | 
 | ||||||
|  |     const packageConfigs = collectPackages(ANGULAR_DIST_PACKAGES_DIR); | ||||||
| 
 | 
 | ||||||
|     this._log('Found the following Angular distributables:', Object.keys(packageConfigs).map(key => `\n - ${key}`)); |     this._log('Found the following Angular distributables:', Object.keys(packageConfigs).map(key => `\n - ${key}`)); | ||||||
|     return packageConfigs; |     return packageConfigs; | ||||||
|  | |||||||
| @ -66,7 +66,7 @@ describe('NgPackagesInstaller', () => { | |||||||
|       // These are the packages that are "found" in the dist directory
 |       // These are the packages that are "found" in the dist directory
 | ||||||
|       dummyNgPackages = { |       dummyNgPackages = { | ||||||
|         '@angular/core': { |         '@angular/core': { | ||||||
|           parentDir: packagesDir, |           packageDir: `${packagesDir}/core`, | ||||||
|           packageJsonPath: `${packagesDir}/core/package.json`, |           packageJsonPath: `${packagesDir}/core/package.json`, | ||||||
|           config: { |           config: { | ||||||
|             peerDependencies: { |             peerDependencies: { | ||||||
| @ -77,17 +77,17 @@ describe('NgPackagesInstaller', () => { | |||||||
|           } |           } | ||||||
|         }, |         }, | ||||||
|         '@angular/common': { |         '@angular/common': { | ||||||
|           parentDir: packagesDir, |           packageDir: `${packagesDir}/common`, | ||||||
|           packageJsonPath: `${packagesDir}/common/package.json`, |           packageJsonPath: `${packagesDir}/common/package.json`, | ||||||
|           config: { peerDependencies: { '@angular/core': '4.4.4-1ab23cd4' } } |           config: { peerDependencies: { '@angular/core': '4.4.4-1ab23cd4' } } | ||||||
|         }, |         }, | ||||||
|         '@angular/compiler': { |         '@angular/compiler': { | ||||||
|           parentDir: packagesDir, |           packageDir: `${packagesDir}/compiler`, | ||||||
|           packageJsonPath: `${packagesDir}/compiler/package.json`, |           packageJsonPath: `${packagesDir}/compiler/package.json`, | ||||||
|           config: { peerDependencies: { '@angular/common': '4.4.4-1ab23cd4' } } |           config: { peerDependencies: { '@angular/common': '4.4.4-1ab23cd4' } } | ||||||
|         }, |         }, | ||||||
|         '@angular/compiler-cli': { |         '@angular/compiler-cli': { | ||||||
|           parentDir: toolsDir, |           packageDir: `${toolsDir}/compiler-cli`, | ||||||
|           packageJsonPath: `${toolsDir}/compiler-cli/package.json`, |           packageJsonPath: `${toolsDir}/compiler-cli/package.json`, | ||||||
|           config: { |           config: { | ||||||
|             dependencies: { '@angular/tsc-wrapped': '4.4.4-1ab23cd4' }, |             dependencies: { '@angular/tsc-wrapped': '4.4.4-1ab23cd4' }, | ||||||
| @ -95,7 +95,7 @@ describe('NgPackagesInstaller', () => { | |||||||
|           } |           } | ||||||
|         }, |         }, | ||||||
|         '@angular/tsc-wrapped': { |         '@angular/tsc-wrapped': { | ||||||
|           parentDir: toolsDir, |           packageDir: `${toolsDir}/tsc-wrapped`, | ||||||
|           packageJsonPath: `${toolsDir}/tsc-wrapped/package.json`, |           packageJsonPath: `${toolsDir}/tsc-wrapped/package.json`, | ||||||
|           config: { |           config: { | ||||||
|             devDependencies: { '@angular/common': '4.4.4-1ab23cd4' }, |             devDependencies: { '@angular/common': '4.4.4-1ab23cd4' }, | ||||||
| @ -288,7 +288,10 @@ describe('NgPackagesInstaller', () => { | |||||||
|   }); |   }); | ||||||
| 
 | 
 | ||||||
|   describe('_getDistPackages()', () => { |   describe('_getDistPackages()', () => { | ||||||
|     beforeEach(() => spyOn(NgPackagesInstaller.prototype, '_buildDistPackages')); |     beforeEach(() => { | ||||||
|  |       fs.existsSync.and.callThrough(); | ||||||
|  |       spyOn(NgPackagesInstaller.prototype, '_buildDistPackages'); | ||||||
|  |     }); | ||||||
| 
 | 
 | ||||||
|     it('should not build the local packages by default', () => { |     it('should not build the local packages by default', () => { | ||||||
|       installer._getDistPackages(); |       installer._getDistPackages(); | ||||||
| @ -309,7 +312,7 @@ describe('NgPackagesInstaller', () => { | |||||||
|     it('should include top level Angular packages', () => { |     it('should include top level Angular packages', () => { | ||||||
|       const ngPackages = installer._getDistPackages(); |       const ngPackages = installer._getDistPackages(); | ||||||
|       const expectedValue = jasmine.objectContaining({ |       const expectedValue = jasmine.objectContaining({ | ||||||
|         parentDir: jasmine.any(String), |         packageDir: jasmine.any(String), | ||||||
|         packageJsonPath: jasmine.any(String), |         packageJsonPath: jasmine.any(String), | ||||||
|         config: jasmine.any(Object), |         config: jasmine.any(Object), | ||||||
|       }); |       }); | ||||||
| @ -323,12 +326,12 @@ describe('NgPackagesInstaller', () => { | |||||||
|       expect(ngPackages['@angular/upgrade/static']).not.toBeDefined(); |       expect(ngPackages['@angular/upgrade/static']).not.toBeDefined(); | ||||||
|     }); |     }); | ||||||
| 
 | 
 | ||||||
|     it('should store each package\'s parent directory', () => { |     it('should store each package\'s directory', () => { | ||||||
|       const ngPackages = installer._getDistPackages(); |       const ngPackages = installer._getDistPackages(); | ||||||
| 
 | 
 | ||||||
|       // For example...
 |       // For example...
 | ||||||
|       expect(ngPackages['@angular/core'].parentDir).toBe(packagesDir); |       expect(ngPackages['@angular/core'].packageDir).toBe(path.join(packagesDir, 'core')); | ||||||
|       expect(ngPackages['@angular/router'].parentDir).toBeDefined(toolsDir); |       expect(ngPackages['@angular/router'].packageDir).toBe(path.join(packagesDir, 'router')); | ||||||
|     }); |     }); | ||||||
| 
 | 
 | ||||||
|     it('should not include packages that have been ignored', () => { |     it('should not include packages that have been ignored', () => { | ||||||
|  | |||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user