fix(ngcc): do not fail hard when a format-path points to a non-existing or empty file (#40985)

Previously, when `ngcc` encountered an entry-point with a format-path
that pointed to a non-existing or empty file it would throw an error and
stop processing the remaining tasks.

In the past, we used to ignore such format-paths and continue processing
the rest of the tasks ([see code][1]). This was changed to a hard
failure in 2954d1b5ca. Looking at the code
history, the reason for changing the behavior was an (incorrect)
assumption that the condition could not fail. This assumption failed to
take into account the case where a 3rd-party library has an invalid
format-path in its `package.json`. This is an issue with the library,
but it should not prevent `ngcc` from processing other
packages/entry-points/formats.

This commit fixes this by reporting the task as failed but not throwing
an error, thus allowing `ngcc` to continue processing other tasks.

[1]: https://github.com/angular/angular/blob/3077c9a1f89c5bd75fb96c16e/packages/compiler-cli/ngcc/src/main.ts#L124

Fixes #40965

PR Close #40985
This commit is contained in:
George Kalpakas 2021-02-25 11:36:17 +02:00 committed by Zach Arend
parent 953e98e211
commit 284af7308b
3 changed files with 90 additions and 10 deletions

View File

@ -46,12 +46,13 @@ export function getCreateCompileFn(
// (i.e. they are defined in `entryPoint.packageJson`). Furthermore, they are also guaranteed // (i.e. they are defined in `entryPoint.packageJson`). Furthermore, they are also guaranteed
// to be among `SUPPORTED_FORMAT_PROPERTIES`. // to be among `SUPPORTED_FORMAT_PROPERTIES`.
// Based on the above, `formatPath` should always be defined and `getEntryPointFormat()` // Based on the above, `formatPath` should always be defined and `getEntryPointFormat()`
// should always return a format here (and not `undefined`). // should always return a format here (and not `undefined`) unless `formatPath` points to a
// missing or empty file.
if (!formatPath || !format) { if (!formatPath || !format) {
// This should never happen. onTaskCompleted(
throw new Error( task, TaskProcessingOutcome.Failed,
`Invariant violated: No format-path or format for ${entryPoint.path} : ` + `property \`${formatProperty}\` pointing to a missing or empty file: ${formatPath}`);
`${formatProperty} (formatPath: ${formatPath} | format: ${format})`); return;
} }
logger.info(`Compiling ${entryPoint.name} : ${formatProperty} as ${format}`); logger.info(`Compiling ${entryPoint.name} : ${formatProperty} as ${format}`);

View File

@ -82,8 +82,8 @@ export function createLogErrorHandler(
} }
function createErrorMessage(fs: ReadonlyFileSystem, task: Task, message: string|null): string { function createErrorMessage(fs: ReadonlyFileSystem, task: Task, message: string|null): string {
const jsFormat = const jsFormat = `\`${task.formatProperty}\` as ${
`${task.formatProperty} as ${getEntryPointFormat(fs, task.entryPoint, task.formatProperty)}`; getEntryPointFormat(fs, task.entryPoint, task.formatProperty) ?? 'unknown format'}`;
const format = task.typingsOnly ? `typings only using ${jsFormat}` : jsFormat; const format = task.typingsOnly ? `typings only using ${jsFormat}` : jsFormat;
message = message !== null ? ` due to ${message}` : ''; message = message !== null ? ` due to ${message}` : '';
return `Failed to compile entry-point ${task.entryPoint.name} (${format})` + message; return `Failed to compile entry-point ${task.entryPoint.name} (${format})` + message;

View File

@ -154,6 +154,85 @@ runInEachFileSystem(() => {
expect(loadPackage('local-package', _('/dist')).__processed_by_ivy_ngcc__).toBeUndefined(); expect(loadPackage('local-package', _('/dist')).__processed_by_ivy_ngcc__).toBeUndefined();
}); });
it('should report an error, if one of the format-paths is missing or empty', () => {
loadTestFiles([
// A package with a format-path (main) that points to a missing file.
{
name: _(`/dist/pkg-with-missing-main/package.json`),
contents: `
{
"name": "pkg-with-missing-main",
"typings": "./index.d.ts",
"es2015": "./index-es2015.js",
"fesm5": "./index-es5.js",
"main": "./index-missing.js"
}
`,
},
{
name: _('/dist/pkg-with-missing-main/index.d.ts'),
contents: 'export type DummyData = boolean;'
},
{
name: _('/dist/pkg-with-missing-main/index-es2015.js'),
contents: 'var DUMMY_DATA = true;'
},
{name: _('/dist/pkg-with-missing-main/index-es5.js'), contents: 'var DUMMY_DATA = true;'},
{name: _('/dist/pkg-with-missing-main/index.metadata.json'), contents: 'DUMMY DATA'},
// A package with a format-path (main) that points to an empty file.
{
name: _(`/dist/pkg-with-empty-main/package.json`),
contents: `
{
"name": "pkg-with-empty-main",
"typings": "./index.d.ts",
"es2015": "./index-es2015.js",
"fesm5": "./index-es5.js",
"main": "./index-empty.js"
}
`,
},
{
name: _('/dist/pkg-with-empty-main/index.d.ts'),
contents: 'export type DummyData = boolean;'
},
{name: _('/dist/pkg-with-empty-main/index-empty.js'), contents: ''},
{name: _('/dist/pkg-with-empty-main/index-es2015.js'), contents: 'var DUMMY_DATA = true;'},
{name: _('/dist/pkg-with-empty-main/index-es5.js'), contents: 'var DUMMY_DATA = true;'},
{name: _('/dist/pkg-with-empty-main/index.metadata.json'), contents: 'DUMMY DATA'},
]);
const logger = new MockLogger();
mainNgcc({
basePath: '/dist',
propertiesToConsider: ['es2015', 'main', 'fesm5'],
logger,
});
expect(loadPackage('pkg-with-missing-main', _('/dist')).__processed_by_ivy_ngcc__).toEqual({
es2015: jasmine.any(String),
fesm5: jasmine.any(String),
typings: jasmine.any(String),
});
expect(loadPackage('pkg-with-empty-main', _('/dist')).__processed_by_ivy_ngcc__).toEqual({
es2015: jasmine.any(String),
fesm5: jasmine.any(String),
typings: jasmine.any(String),
});
expect(logger.logs.error).toEqual([
[
'Failed to compile entry-point pkg-with-missing-main (`main` as unknown format) due to ' +
'property `main` pointing to a missing or empty file: ./index-missing.js',
],
[
'Failed to compile entry-point pkg-with-empty-main (`main` as unknown format) due to ' +
'property `main` pointing to a missing or empty file: ./index-empty.js',
],
]);
});
it('should generate correct metadata for decorated getter/setter properties', () => { it('should generate correct metadata for decorated getter/setter properties', () => {
setupAngularCoreEsm5(); setupAngularCoreEsm5();
compileIntoFlatEs5Package('test-package', { compileIntoFlatEs5Package('test-package', {
@ -573,7 +652,7 @@ runInEachFileSystem(() => {
fail('should have thrown'); fail('should have thrown');
} catch (e) { } catch (e) {
expect(e.message).toContain( expect(e.message).toContain(
'Failed to compile entry-point test-package (esm2015 as esm2015) due to compilation errors:'); 'Failed to compile entry-point test-package (`esm2015` as esm2015) due to compilation errors:');
expect(e.message).toContain('NG1010'); expect(e.message).toContain('NG1010');
expect(e.message).toContain('selector must be a string'); expect(e.message).toContain('selector must be a string');
} }
@ -1552,7 +1631,7 @@ runInEachFileSystem(() => {
fail('should have thrown'); fail('should have thrown');
} catch (e) { } catch (e) {
expect(e.message).toContain( expect(e.message).toContain(
'Failed to compile entry-point fatal-error (es2015 as esm2015) due to compilation errors:'); 'Failed to compile entry-point fatal-error (`es2015` as esm2015) due to compilation errors:');
expect(e.message).toContain('NG2001'); expect(e.message).toContain('NG2001');
expect(e.message).toContain('component is missing a template'); expect(e.message).toContain('component is missing a template');
} }
@ -1630,7 +1709,7 @@ runInEachFileSystem(() => {
expect(logger.logs.error.length).toEqual(1); expect(logger.logs.error.length).toEqual(1);
const message = logger.logs.error[0][0]; const message = logger.logs.error[0][0];
expect(message).toContain( expect(message).toContain(
'Failed to compile entry-point fatal-error (es2015 as esm2015) due to compilation errors:'); 'Failed to compile entry-point fatal-error (`es2015` as esm2015) due to compilation errors:');
expect(message).toContain('NG2001'); expect(message).toContain('NG2001');
expect(message).toContain('component is missing a template'); expect(message).toContain('component is missing a template');