fix(ngcc): update package.json deterministically (#34870)

Ngcc adds properties to the `package.json` files of the entry-points it
processes to mark them as processed for a format and point to the
created Ivy entry-points (in case of `--create-ivy-entry-points`). When
running ngcc in parallel mode (which is the default for the standalone
ngcc command), multiple formats can be processed simultaneously for the
same entry-point and the order of completion is not deterministic.

Previously, ngcc would append new properties at the end of the target
object in `package.json` as soon as the format processing was completed.
As a result, the order of properties in the resulting `package.json`
(when processing multiple formats for an entry-point in parallel) was
not deterministic. For tools that use file hashes for caching purposes
(such as Bazel), this lead to a high probability of cache misses.

This commit fixes the problem by ensuring that the position of
properties added to `package.json` files is deterministic and
independent of the order in which each format is processed.

Jira issue: [FW-1801](https://angular-team.atlassian.net/browse/FW-1801)

Fixes #34635

PR Close #34870
This commit is contained in:
George Kalpakas
2020-01-20 18:02:12 +02:00
committed by Andrew Kushnir
parent 8215b345e3
commit a10d2a8dc4
7 changed files with 314 additions and 26 deletions

View File

@ -14,7 +14,7 @@ import {absoluteFrom as _} from '../../../../src/ngtsc/file_system';
import {runInEachFileSystem} from '../../../../src/ngtsc/file_system/testing';
import {ClusterPackageJsonUpdater} from '../../../src/execution/cluster/package_json_updater';
import {JsonObject} from '../../../src/packages/entry_point';
import {PackageJsonUpdate, PackageJsonUpdater} from '../../../src/writing/package_json_updater';
import {PackageJsonPropertyPositioning, PackageJsonUpdate, PackageJsonUpdater} from '../../../src/writing/package_json_updater';
import {mockProperty} from '../../helpers/spy_utils';
@ -48,10 +48,18 @@ runInEachFileSystem(() => {
const update = updater.createUpdate();
update.addChange(['foo'], 'updated');
update.addChange(['baz'], 'updated 2', 'alphabetic');
update.addChange(['bar'], 'updated 3', {before: 'bar'});
update.writeChanges(jsonPath);
expect(writeChangesSpy)
.toHaveBeenCalledWith([[['foo'], 'updated']], jsonPath, undefined);
.toHaveBeenCalledWith(
[
[['foo'], 'updated', 'unimportant'],
[['baz'], 'updated 2', 'alphabetic'],
[['bar'], 'updated 3', {before: 'bar'}],
],
jsonPath, undefined);
});
}));
});
@ -65,10 +73,18 @@ runInEachFileSystem(() => {
const jsonPath = _('/foo/package.json');
const parsedJson = {foo: 'bar'};
updater.createUpdate().addChange(['foo'], 'updated').writeChanges(jsonPath, parsedJson);
updater.createUpdate()
.addChange(['foo'], 'updated')
.addChange(['bar'], 'updated too', 'alphabetic')
.writeChanges(jsonPath, parsedJson);
expect(delegate.writeChanges)
.toHaveBeenCalledWith([[['foo'], 'updated']], jsonPath, parsedJson);
.toHaveBeenCalledWith(
[
[['foo'], 'updated', 'unimportant'],
[['bar'], 'updated too', 'alphabetic'],
],
jsonPath, parsedJson);
});
it('should throw, if trying to re-apply an already applied update', () => {
@ -89,21 +105,31 @@ runInEachFileSystem(() => {
it('should send an `update-package-json` message to the master process', () => {
const jsonPath = _('/foo/package.json');
const writeToProp = (propPath: string[], parsed?: JsonObject) =>
updater.createUpdate().addChange(propPath, 'updated').writeChanges(jsonPath, parsed);
const writeToProp =
(propPath: string[], positioning?: PackageJsonPropertyPositioning,
parsed?: JsonObject) => updater.createUpdate()
.addChange(propPath, 'updated', positioning)
.writeChanges(jsonPath, parsed);
writeToProp(['foo']);
expect(processSendSpy).toHaveBeenCalledWith({
type: 'update-package-json',
packageJsonPath: jsonPath,
changes: [[['foo'], 'updated']],
changes: [[['foo'], 'updated', 'unimportant']],
});
writeToProp(['bar', 'baz', 'qux'], {});
writeToProp(['bar'], {before: 'foo'});
expect(processSendSpy).toHaveBeenCalledWith({
type: 'update-package-json',
packageJsonPath: jsonPath,
changes: [[['bar', 'baz', 'qux'], 'updated']],
changes: [[['bar'], 'updated', {before: 'foo'}]],
});
writeToProp(['bar', 'baz', 'qux'], 'alphabetic', {});
expect(processSendSpy).toHaveBeenCalledWith({
type: 'update-package-json',
packageJsonPath: jsonPath,
changes: [[['bar', 'baz', 'qux'], 'updated', 'alphabetic']],
});
});