Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(compiler-cli): ngcc - cope with processing entry-points multiple … #29657

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -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];
@@ -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);
@@ -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');
}
}
}

@@ -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({
@@ -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({
@@ -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.
@@ -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',
});
});
});
@@ -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',
});
});
This conversation was marked as resolved by petebacondarwin

This comment has been minimized.

Copy link
@gkalpak

gkalpak Apr 2, 2019

Member

Does this test verify that typings are not processed again? (If so, how? 😕)

This comment has been minimized.

Copy link
@petebacondarwin

petebacondarwin Apr 2, 2019

Author Member

If you process the typings a second time it tries to write the compiled files, but this throws an error because there are already bak files in place. This test shows that we can get a new format compiled without throwing an error.

This comment has been minimized.

Copy link
@gkalpak

gkalpak Apr 2, 2019

Member

👍
(Might be worth mentioning that in a comment, because it is not obvious 😁)

This comment has been minimized.

Copy link
@petebacondarwin

petebacondarwin Apr 2, 2019

Author Member

DONE

});

describe('with createNewEntryPointFormats', () => {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.