Skip to content

Commit

Permalink
fix(compiler-cli): ngcc - cope with processing entry-points multiple …
Browse files Browse the repository at this point in the history
…times (#29657)

With the new API, where you can choose to only process the first
matching format, it is possible to process an entry-point multiple
times, if you pass in a different format each time.

Previously, ngcc would always try to process the typings files for
the entry-point along with processing the first format of the current
execution of ngcc. But this meant that it would be trying to process
the typings a second time.

Now we only process the typings if they have not already been
processed as part of processing another format in another
even if it was in a different execution of ngcc.

PR Close #29657
  • Loading branch information
petebacondarwin authored and jasonaden committed Apr 2, 2019
1 parent c579949 commit 6b39c9c
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 3 deletions.
13 changes: 10 additions & 3 deletions packages/compiler-cli/ngcc/src/main.ts
Expand Up @@ -110,6 +110,8 @@ export function mainNgcc({basePath, targetEntryPointPath,
const entryPointPackageJson = entryPoint.packageJson;
const entryPointPackageJsonPath = AbsoluteFsPath.from(resolve(entryPoint.path, 'package.json'));

const hasProcessedDts = hasBeenProcessed(entryPointPackageJson, 'typings');

for (let i = 0; i < propertiesToConsider.length; i++) {
const property = propertiesToConsider[i] as EntryPointJsonProperty;
const formatPath = entryPointPackageJson[property];
Expand All @@ -124,12 +126,14 @@ export function mainNgcc({basePath, targetEntryPointPath,
continue;
}

const isFirstFormat = compiledFormats.size === 0;
const processDts = !hasProcessedDts && isFirstFormat;

// We don't break if this if statement fails because we still want to mark
// the property as processed even if its underlying format has been built already.
if (!compiledFormats.has(formatPath) && (compileAllFormats || compiledFormats.size === 0)) {
if (!compiledFormats.has(formatPath) && (compileAllFormats || isFirstFormat)) {
const bundle = makeEntryPointBundle(
entryPoint.path, formatPath, entryPoint.typings, isCore, property, format,
compiledFormats.size === 0);
entryPoint.path, formatPath, entryPoint.typings, isCore, property, format, processDts);
if (bundle) {
logger.info(`Compiling ${entryPoint.name} : ${property} as ${format}`);
const transformedFiles = transformer.transform(bundle);
Expand All @@ -147,6 +151,9 @@ export function mainNgcc({basePath, targetEntryPointPath,
// previous property.
if (compiledFormats.has(formatPath)) {
markAsProcessed(entryPointPackageJson, entryPointPackageJsonPath, property);
if (processDts) {
markAsProcessed(entryPointPackageJson, entryPointPackageJsonPath, 'typings');
}
}
}

Expand Down
32 changes: 32 additions & 0 deletions packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts
Expand Up @@ -41,6 +41,7 @@ describe('ngcc main()', () => {
esm2015: '0.0.0-PLACEHOLDER',
fesm5: '0.0.0-PLACEHOLDER',
fesm2015: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
// * `common` is a dependency of `common/http`, so is compiled.
expect(loadPackage('@angular/common').__processed_by_ivy_ngcc__).toEqual({
Expand All @@ -50,6 +51,7 @@ describe('ngcc main()', () => {
esm2015: '0.0.0-PLACEHOLDER',
fesm5: '0.0.0-PLACEHOLDER',
fesm2015: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
// * `core` is a dependency of `common`, so is compiled.
expect(loadPackage('@angular/core').__processed_by_ivy_ngcc__).toEqual({
Expand All @@ -59,6 +61,7 @@ describe('ngcc main()', () => {
esm2015: '0.0.0-PLACEHOLDER',
fesm5: '0.0.0-PLACEHOLDER',
fesm2015: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});

// * `common/testing` is not a dependency of `common/http` so is not compiled.
Expand Down Expand Up @@ -94,21 +97,25 @@ describe('ngcc main()', () => {
esm5: '0.0.0-PLACEHOLDER',
module: '0.0.0-PLACEHOLDER',
fesm5: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
expect(loadPackage('@angular/common').__processed_by_ivy_ngcc__).toEqual({
esm5: '0.0.0-PLACEHOLDER',
module: '0.0.0-PLACEHOLDER',
fesm5: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
expect(loadPackage('@angular/common/testing').__processed_by_ivy_ngcc__).toEqual({
esm5: '0.0.0-PLACEHOLDER',
module: '0.0.0-PLACEHOLDER',
fesm5: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
expect(loadPackage('@angular/common/http').__processed_by_ivy_ngcc__).toEqual({
esm5: '0.0.0-PLACEHOLDER',
module: '0.0.0-PLACEHOLDER',
fesm5: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
});
});
Expand All @@ -127,20 +134,45 @@ describe('ngcc main()', () => {
expect(loadPackage('@angular/core').__processed_by_ivy_ngcc__).toEqual({
fesm5: '0.0.0-PLACEHOLDER',
module: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
expect(loadPackage('@angular/common').__processed_by_ivy_ngcc__).toEqual({
fesm5: '0.0.0-PLACEHOLDER',
module: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
expect(loadPackage('@angular/common/testing').__processed_by_ivy_ngcc__).toEqual({
fesm5: '0.0.0-PLACEHOLDER',
module: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
expect(loadPackage('@angular/common/http').__processed_by_ivy_ngcc__).toEqual({
fesm5: '0.0.0-PLACEHOLDER',
module: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
});

it('should cope with compiling the same entry-point multiple times with different formats',
() => {
mainNgcc({
basePath: '/node_modules',
propertiesToConsider: ['module'],
compileAllFormats: false
});
expect(loadPackage('@angular/core').__processed_by_ivy_ngcc__).toEqual({
module: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
// If ngcc tries to write out the typings files again, this will throw an exception.
mainNgcc(
{basePath: '/node_modules', propertiesToConsider: ['esm5'], compileAllFormats: false});
expect(loadPackage('@angular/core').__processed_by_ivy_ngcc__).toEqual({
esm5: '0.0.0-PLACEHOLDER',
module: '0.0.0-PLACEHOLDER',
typings: '0.0.0-PLACEHOLDER',
});
});
});

describe('with createNewEntryPointFormats', () => {
Expand Down

0 comments on commit 6b39c9c

Please sign in to comment.