From cc43bfa725736d6d54f8bb4d75f783227430dba6 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Fri, 31 Jan 2020 21:07:58 +0000 Subject: [PATCH] refactor(ngcc): do not crash if package build version is outdated (#35079) Now `hasBeenProcessed()` will no longer throw if there is an entry-point that has been built with an outdated version of ngcc. Instead it just returns `false`, which will include it in this processing run. This is a precursor to adding functionality that will automatically revert outdate build artifacts. PR Close #35079 --- packages/compiler-cli/ngcc/src/main.ts | 6 +- .../ngcc/src/packages/build_marker.ts | 30 ++----- .../ngcc/test/packages/build_marker_spec.ts | 83 +------------------ 3 files changed, 13 insertions(+), 106 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/main.ts b/packages/compiler-cli/ngcc/src/main.ts index c925df35aa..38f7b9c512 100644 --- a/packages/compiler-cli/ngcc/src/main.ts +++ b/packages/compiler-cli/ngcc/src/main.ts @@ -198,7 +198,7 @@ export function mainNgcc( for (const entryPoint of entryPoints) { const packageJson = entryPoint.packageJson; - const hasProcessedTypings = hasBeenProcessed(packageJson, 'typings', entryPoint.path); + const hasProcessedTypings = hasBeenProcessed(packageJson, 'typings'); const {propertiesToProcess, equivalentPropertiesMap} = getPropertiesToProcess(packageJson, supportedPropertiesToConsider, compileAllFormats); let processDts = !hasProcessedTypings; @@ -263,7 +263,7 @@ export function mainNgcc( } // The format-path which the property maps to is already processed - nothing to do. - if (hasBeenProcessed(packageJson, formatProperty, entryPoint.path)) { + if (hasBeenProcessed(packageJson, formatProperty)) { logger.debug(`Skipping ${entryPoint.name} : ${formatProperty} (already compiled).`); onTaskCompleted(task, TaskProcessingOutcome.AlreadyProcessed); return; @@ -411,7 +411,7 @@ function hasProcessedTargetEntryPoint( for (const property of propertiesToConsider) { if (packageJson[property]) { // Here is a property that should be processed - if (hasBeenProcessed(packageJson, property as EntryPointJsonProperty, targetPath)) { + if (hasBeenProcessed(packageJson, property as EntryPointJsonProperty)) { if (!compileAllFormats) { // It has been processed and we only need one, so we are done. return true; diff --git a/packages/compiler-cli/ngcc/src/packages/build_marker.ts b/packages/compiler-cli/ngcc/src/packages/build_marker.ts index 61caa52e32..140a3efc6b 100644 --- a/packages/compiler-cli/ngcc/src/packages/build_marker.ts +++ b/packages/compiler-cli/ngcc/src/packages/build_marker.ts @@ -5,7 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {AbsoluteFsPath, basename, dirname, isRoot} from '../../../src/ngtsc/file_system'; +import {AbsoluteFsPath} from '../../../src/ngtsc/file_system'; import {PackageJsonUpdater} from '../writing/package_json_updater'; import {EntryPointPackageJson, PackageJsonFormatProperties} from './entry_point'; @@ -14,33 +14,15 @@ export const NGCC_VERSION = '0.0.0-PLACEHOLDER'; /** * Check whether ngcc has already processed a given entry-point format. * - * The entry-point is defined by the package.json contents provided. - * The format is defined by the provided property name of the path to the bundle in the package.json - * * @param packageJson The parsed contents of the package.json file for the entry-point. * @param format The entry-point format property in the package.json to check. - * @returns true if the entry-point and format have already been processed with this ngcc version. - * @throws Error if the `packageJson` property is not an object. - * @throws Error if the entry-point has already been processed with a different ngcc version. + * @returns true if the `format` in the entry-point has already been processed by this ngcc version, + * false otherwise. */ export function hasBeenProcessed( - packageJson: EntryPointPackageJson, format: PackageJsonFormatProperties, - entryPointPath: AbsoluteFsPath): boolean { - if (!packageJson.__processed_by_ivy_ngcc__) { - return false; - } - if (Object.keys(packageJson.__processed_by_ivy_ngcc__) - .some(property => packageJson.__processed_by_ivy_ngcc__ ![property] !== NGCC_VERSION)) { - let nodeModulesFolderPath = entryPointPath; - while (!isRoot(nodeModulesFolderPath) && basename(nodeModulesFolderPath) !== 'node_modules') { - nodeModulesFolderPath = dirname(nodeModulesFolderPath); - } - throw new Error( - `The ngcc compiler has changed since the last ngcc build.\n` + - `Please remove "${isRoot(nodeModulesFolderPath) ? entryPointPath : nodeModulesFolderPath}" and try again.`); - } - - return packageJson.__processed_by_ivy_ngcc__[format] === NGCC_VERSION; + packageJson: EntryPointPackageJson, format: PackageJsonFormatProperties): boolean { + return packageJson.__processed_by_ivy_ngcc__ !== undefined && + packageJson.__processed_by_ivy_ngcc__[format] === NGCC_VERSION; } /** diff --git a/packages/compiler-cli/ngcc/test/packages/build_marker_spec.ts b/packages/compiler-cli/ngcc/test/packages/build_marker_spec.ts index 1f0e145d45..9c729341ac 100644 --- a/packages/compiler-cli/ngcc/test/packages/build_marker_spec.ts +++ b/packages/compiler-cli/ngcc/test/packages/build_marker_spec.ts @@ -5,7 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {AbsoluteFsPath, absoluteFrom, getFileSystem} from '../../../src/ngtsc/file_system'; +import {absoluteFrom, getFileSystem} from '../../../src/ngtsc/file_system'; import {runInEachFileSystem} from '../../../src/ngtsc/file_system/testing'; import {loadTestFiles} from '../../../test/helpers'; import {hasBeenProcessed, markAsProcessed} from '../../src/packages/build_marker'; @@ -177,97 +177,22 @@ runInEachFileSystem(() => { }); describe('hasBeenProcessed', () => { - let entryPointPath: AbsoluteFsPath; - let nodeModulesPath: AbsoluteFsPath; - - beforeEach(() => { - entryPointPath = _('/node_modules/test'); - nodeModulesPath = _('/node_modules'); - }); - it('should return true if the marker exists for the given format property', () => { expect(hasBeenProcessed( {name: 'test', __processed_by_ivy_ngcc__: {'fesm2015': '0.0.0-PLACEHOLDER'}}, - 'fesm2015', entryPointPath)) + 'fesm2015')) .toBe(true); }); it('should return false if the marker does not exist for the given format property', () => { expect(hasBeenProcessed( {name: 'test', __processed_by_ivy_ngcc__: {'fesm2015': '0.0.0-PLACEHOLDER'}}, - 'module', entryPointPath)) + 'module')) .toBe(false); }); it('should return false if no markers exist', - () => { expect(hasBeenProcessed({name: 'test'}, 'module', entryPointPath)).toBe(false); }); - - it('should throw an Error if the format has been compiled with a different version.', () => { - expect( - () => hasBeenProcessed( - {name: 'test', __processed_by_ivy_ngcc__: {'fesm2015': '8.0.0'}}, 'fesm2015', - entryPointPath)) - .toThrowError( - 'The ngcc compiler has changed since the last ngcc build.\n' + - `Please remove "${nodeModulesPath}" and try again.`); - }); - - it('should throw an Error if any format has been compiled with a different version.', () => { - expect( - () => hasBeenProcessed( - {name: 'test', __processed_by_ivy_ngcc__: {'fesm2015': '8.0.0'}}, 'module', - entryPointPath)) - .toThrowError( - 'The ngcc compiler has changed since the last ngcc build.\n' + - `Please remove "${nodeModulesPath}" and try again.`); - expect( - () => hasBeenProcessed( - { - name: 'test', - __processed_by_ivy_ngcc__: {'module': '0.0.0-PLACEHOLDER', 'fesm2015': '8.0.0'} - }, - 'module', entryPointPath)) - .toThrowError( - 'The ngcc compiler has changed since the last ngcc build.\n' + - `Please remove "${nodeModulesPath}" and try again.`); - expect( - () => hasBeenProcessed( - { - name: 'test', - __processed_by_ivy_ngcc__: {'module': '0.0.0-PLACEHOLDER', 'fesm2015': '8.0.0'} - }, - 'fesm2015', entryPointPath)) - .toThrowError( - 'The ngcc compiler has changed since the last ngcc build.\n' + - `Please remove "${nodeModulesPath}" and try again.`); - }); - - it('should throw an Error, with the appropriate path to remove, if the format has been compiled with a different version', - () => { - expect( - () => hasBeenProcessed( - {name: 'test', __processed_by_ivy_ngcc__: {'fesm2015': '8.0.0'}}, 'fesm2015', - _('/node_modules/test'))) - .toThrowError( - 'The ngcc compiler has changed since the last ngcc build.\n' + - `Please remove "${_('/node_modules')}" and try again.`); - - expect( - () => hasBeenProcessed( - {name: 'nested', __processed_by_ivy_ngcc__: {'fesm2015': '8.0.0'}}, 'fesm2015', - _('/node_modules/test/node_modules/nested'))) - .toThrowError( - 'The ngcc compiler has changed since the last ngcc build.\n' + - `Please remove "${_('/node_modules/test/node_modules')}" and try again.`); - - expect( - () => hasBeenProcessed( - {name: 'test', __processed_by_ivy_ngcc__: {'fesm2015': '8.0.0'}}, 'fesm2015', - _('/dist/test'))) - .toThrowError( - 'The ngcc compiler has changed since the last ngcc build.\n' + - `Please remove "${_('/dist/test')}" and try again.`); - }); + () => { expect(hasBeenProcessed({name: 'test'}, 'module')).toBe(false); }); }); }); });