Skip to content

Commit 06ff277

Browse files
gkalpakzarend
gkalpak
authored and
zarend
committed
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 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
1 parent 71ef11f commit 06ff277

File tree

3 files changed

+90
-10
lines changed

3 files changed

+90
-10
lines changed

‎packages/compiler-cli/ngcc/src/execution/create_compile_function.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,13 @@ export function getCreateCompileFn(
4646
// (i.e. they are defined in `entryPoint.packageJson`). Furthermore, they are also guaranteed
4747
// to be among `SUPPORTED_FORMAT_PROPERTIES`.
4848
// Based on the above, `formatPath` should always be defined and `getEntryPointFormat()`
49-
// should always return a format here (and not `undefined`).
49+
// should always return a format here (and not `undefined`) unless `formatPath` points to a
50+
// missing or empty file.
5051
if (!formatPath || !format) {
51-
// This should never happen.
52-
throw new Error(
53-
`Invariant violated: No format-path or format for ${entryPoint.path} : ` +
54-
`${formatProperty} (formatPath: ${formatPath} | format: ${format})`);
52+
onTaskCompleted(
53+
task, TaskProcessingOutcome.Failed,
54+
`property \`${formatProperty}\` pointing to a missing or empty file: ${formatPath}`);
55+
return;
5556
}
5657

5758
logger.info(`Compiling ${entryPoint.name} : ${formatProperty} as ${format}`);

‎packages/compiler-cli/ngcc/src/execution/tasks/completion.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,8 @@ export function createLogErrorHandler(
8282
}
8383

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

‎packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts‎

Lines changed: 82 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,85 @@ runInEachFileSystem(() => {
154154
expect(loadPackage('local-package', _('/dist')).__processed_by_ivy_ngcc__).toBeUndefined();
155155
});
156156

157+
it('should report an error, if one of the format-paths is missing or empty', () => {
158+
loadTestFiles([
159+
// A package with a format-path (main) that points to a missing file.
160+
{
161+
name: _(`/dist/pkg-with-missing-main/package.json`),
162+
contents: `
163+
{
164+
"name": "pkg-with-missing-main",
165+
"typings": "./index.d.ts",
166+
"es2015": "./index-es2015.js",
167+
"fesm5": "./index-es5.js",
168+
"main": "./index-missing.js"
169+
}
170+
`,
171+
},
172+
{
173+
name: _('/dist/pkg-with-missing-main/index.d.ts'),
174+
contents: 'export type DummyData = boolean;'
175+
},
176+
{
177+
name: _('/dist/pkg-with-missing-main/index-es2015.js'),
178+
contents: 'var DUMMY_DATA = true;'
179+
},
180+
{name: _('/dist/pkg-with-missing-main/index-es5.js'), contents: 'var DUMMY_DATA = true;'},
181+
{name: _('/dist/pkg-with-missing-main/index.metadata.json'), contents: 'DUMMY DATA'},
182+
183+
// A package with a format-path (main) that points to an empty file.
184+
{
185+
name: _(`/dist/pkg-with-empty-main/package.json`),
186+
contents: `
187+
{
188+
"name": "pkg-with-empty-main",
189+
"typings": "./index.d.ts",
190+
"es2015": "./index-es2015.js",
191+
"fesm5": "./index-es5.js",
192+
"main": "./index-empty.js"
193+
}
194+
`,
195+
},
196+
{
197+
name: _('/dist/pkg-with-empty-main/index.d.ts'),
198+
contents: 'export type DummyData = boolean;'
199+
},
200+
{name: _('/dist/pkg-with-empty-main/index-empty.js'), contents: ''},
201+
{name: _('/dist/pkg-with-empty-main/index-es2015.js'), contents: 'var DUMMY_DATA = true;'},
202+
{name: _('/dist/pkg-with-empty-main/index-es5.js'), contents: 'var DUMMY_DATA = true;'},
203+
{name: _('/dist/pkg-with-empty-main/index.metadata.json'), contents: 'DUMMY DATA'},
204+
]);
205+
206+
const logger = new MockLogger();
207+
mainNgcc({
208+
basePath: '/dist',
209+
propertiesToConsider: ['es2015', 'main', 'fesm5'],
210+
logger,
211+
});
212+
213+
expect(loadPackage('pkg-with-missing-main', _('/dist')).__processed_by_ivy_ngcc__).toEqual({
214+
es2015: jasmine.any(String),
215+
fesm5: jasmine.any(String),
216+
typings: jasmine.any(String),
217+
});
218+
expect(loadPackage('pkg-with-empty-main', _('/dist')).__processed_by_ivy_ngcc__).toEqual({
219+
es2015: jasmine.any(String),
220+
fesm5: jasmine.any(String),
221+
typings: jasmine.any(String),
222+
});
223+
224+
expect(logger.logs.error).toEqual([
225+
[
226+
'Failed to compile entry-point pkg-with-missing-main (`main` as unknown format) due to ' +
227+
'property `main` pointing to a missing or empty file: ./index-missing.js',
228+
],
229+
[
230+
'Failed to compile entry-point pkg-with-empty-main (`main` as unknown format) due to ' +
231+
'property `main` pointing to a missing or empty file: ./index-empty.js',
232+
],
233+
]);
234+
});
235+
157236
it('should generate correct metadata for decorated getter/setter properties', () => {
158237
setupAngularCoreEsm5();
159238
compileIntoFlatEs5Package('test-package', {
@@ -573,7 +652,7 @@ runInEachFileSystem(() => {
573652
fail('should have thrown');
574653
} catch (e) {
575654
expect(e.message).toContain(
576-
'Failed to compile entry-point test-package (esm2015 as esm2015) due to compilation errors:');
655+
'Failed to compile entry-point test-package (`esm2015` as esm2015) due to compilation errors:');
577656
expect(e.message).toContain('NG1010');
578657
expect(e.message).toContain('selector must be a string');
579658
}
@@ -1552,7 +1631,7 @@ runInEachFileSystem(() => {
15521631
fail('should have thrown');
15531632
} catch (e) {
15541633
expect(e.message).toContain(
1555-
'Failed to compile entry-point fatal-error (es2015 as esm2015) due to compilation errors:');
1634+
'Failed to compile entry-point fatal-error (`es2015` as esm2015) due to compilation errors:');
15561635
expect(e.message).toContain('NG2001');
15571636
expect(e.message).toContain('component is missing a template');
15581637
}
@@ -1630,7 +1709,7 @@ runInEachFileSystem(() => {
16301709
expect(logger.logs.error.length).toEqual(1);
16311710
const message = logger.logs.error[0][0];
16321711
expect(message).toContain(
1633-
'Failed to compile entry-point fatal-error (es2015 as esm2015) due to compilation errors:');
1712+
'Failed to compile entry-point fatal-error (`es2015` as esm2015) due to compilation errors:');
16341713
expect(message).toContain('NG2001');
16351714
expect(message).toContain('component is missing a template');
16361715

0 commit comments

Comments
 (0)