Skip to content

Commit

Permalink
fix(ngcc): do not fail hard when a format-path points to a non-existi…
Browse files Browse the repository at this point in the history
…ng 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 2954d1b. 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
  • Loading branch information
gkalpak authored and zarend committed Feb 26, 2021
1 parent 71ef11f commit 06ff277
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,13 @@ export function getCreateCompileFn(
// (i.e. they are defined in `entryPoint.packageJson`). Furthermore, they are also guaranteed
// to be among `SUPPORTED_FORMAT_PROPERTIES`.
// 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) {
// This should never happen.
throw new Error(
`Invariant violated: No format-path or format for ${entryPoint.path} : ` +
`${formatProperty} (formatPath: ${formatPath} | format: ${format})`);
onTaskCompleted(
task, TaskProcessingOutcome.Failed,
`property \`${formatProperty}\` pointing to a missing or empty file: ${formatPath}`);
return;
}

logger.info(`Compiling ${entryPoint.name} : ${formatProperty} as ${format}`);
Expand Down
4 changes: 2 additions & 2 deletions packages/compiler-cli/ngcc/src/execution/tasks/completion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ export function createLogErrorHandler(
}

function createErrorMessage(fs: ReadonlyFileSystem, task: Task, message: string|null): string {
const jsFormat =
`${task.formatProperty} as ${getEntryPointFormat(fs, task.entryPoint, task.formatProperty)}`;
const jsFormat = `\`${task.formatProperty}\` as ${
getEntryPointFormat(fs, task.entryPoint, task.formatProperty) ?? 'unknown format'}`;
const format = task.typingsOnly ? `typings only using ${jsFormat}` : jsFormat;
message = message !== null ? ` due to ${message}` : '';
return `Failed to compile entry-point ${task.entryPoint.name} (${format})` + message;
Expand Down
85 changes: 82 additions & 3 deletions packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,85 @@ runInEachFileSystem(() => {
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', () => {
setupAngularCoreEsm5();
compileIntoFlatEs5Package('test-package', {
Expand Down Expand Up @@ -573,7 +652,7 @@ runInEachFileSystem(() => {
fail('should have thrown');
} catch (e) {
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('selector must be a string');
}
Expand Down Expand Up @@ -1552,7 +1631,7 @@ runInEachFileSystem(() => {
fail('should have thrown');
} catch (e) {
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('component is missing a template');
}
Expand Down Expand Up @@ -1630,7 +1709,7 @@ runInEachFileSystem(() => {
expect(logger.logs.error.length).toEqual(1);
const message = logger.logs.error[0][0];
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('component is missing a template');

Expand Down

0 comments on commit 06ff277

Please sign in to comment.