refactor(ngcc): take advantage of early knowledge about format property processability (#32427)
In the past, a task's processability didn't use to be known in advance.
It was possible that a task would be created and added to the queue
during the analysis phase and then later (during the compilation phase)
it would be found out that the task (i.e. the associated format
property) was not processable.
As a result, certain checks had to be delayed, until a task's processing
had started or even until all tasks had been processed. Examples of
checks that had to be delayed are:
- Whether a task can be skipped due to `compileAllFormats: false`.
- Whether there were entry-points for which no format at all was
successfully processed.
It turns out that (as made clear by the refactoring in 9537b2ff8
), once
a task starts being processed it is expected to either complete
successfully (with the associated format being processed) or throw an
error (in which case the process will exit). In other words, a task's
processability is known in advance.
This commit takes advantage of this fact by moving certain checks
earlier in the process (e.g. in the analysis phase instead of the
compilation phase), which in turn allows avoiding some unnecessary work.
More specifically:
- When `compileAllFormats` is `false`, tasks are created _only_ for the
first suitable format property for each entry-point, since the rest of
the tasks would have been skipped during the compilation phase anyway.
This has the following advantages:
1. It avoids the slight overhead of generating extraneous tasks and
then starting to process them (before realizing they should be
skipped).
2. In a potential future parallel execution mode, unnecessary tasks
might start being processed at the same time as the first (useful)
task, even if their output would be later discarded, wasting
resources. Alternatively, extra logic would have to be added to
prevent this from happening. The change in this commit avoids these
issues.
- When an entry-point is not processable, an error will be thrown
upfront without having to wait for other tasks to be processed before
failing.
PR Close #32427
This commit is contained in:
parent
3127ba3c35
commit
9270d3f279
|
@ -20,20 +20,13 @@ export type CompileFn = (task: Task) => void;
|
|||
/** The type of the function that creates the `CompileFn` function used to process tasks. */
|
||||
export type CreateCompileFn = (onTaskCompleted: TaskCompletedCallback) => CompileFn;
|
||||
|
||||
/** Options related to the orchestration/execution of tasks. */
|
||||
export interface ExecutionOptions {
|
||||
compileAllFormats: boolean;
|
||||
propertiesToConsider: string[];
|
||||
}
|
||||
|
||||
/**
|
||||
* A class that orchestrates and executes the required work (i.e. analyzes the entry-points,
|
||||
* processes the resulting tasks, does book-keeping and validates the final outcome).
|
||||
*/
|
||||
export interface Executor {
|
||||
execute(
|
||||
analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn,
|
||||
options: ExecutionOptions): void|Promise<void>;
|
||||
execute(analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn):
|
||||
void|Promise<void>;
|
||||
}
|
||||
|
||||
/** Represents metadata related to the processing of an entry-point. */
|
||||
|
@ -43,12 +36,6 @@ export interface EntryPointProcessingMetadata {
|
|||
* processed).
|
||||
*/
|
||||
hasProcessedTypings: boolean;
|
||||
|
||||
/**
|
||||
* Whether at least one format has been successfully processed (or was already processed) for the
|
||||
* entry-point.
|
||||
*/
|
||||
hasAnyProcessedFormat: boolean;
|
||||
}
|
||||
|
||||
/** Represents a unit of work: processing a specific format property of an entry-point. */
|
||||
|
|
|
@ -9,8 +9,8 @@
|
|||
import {Logger} from '../logging/logger';
|
||||
import {PackageJsonUpdater} from '../writing/package_json_updater';
|
||||
|
||||
import {AnalyzeEntryPointsFn, CreateCompileFn, ExecutionOptions, Executor} from './api';
|
||||
import {checkForUnprocessedEntryPoints, onTaskCompleted} from './utils';
|
||||
import {AnalyzeEntryPointsFn, CreateCompileFn, Executor} from './api';
|
||||
import {onTaskCompleted} from './utils';
|
||||
|
||||
|
||||
/**
|
||||
|
@ -19,9 +19,7 @@ import {checkForUnprocessedEntryPoints, onTaskCompleted} from './utils';
|
|||
export class SingleProcessExecutor implements Executor {
|
||||
constructor(private logger: Logger, private pkgJsonUpdater: PackageJsonUpdater) {}
|
||||
|
||||
execute(
|
||||
analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn,
|
||||
options: ExecutionOptions): void {
|
||||
execute(analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn): void {
|
||||
this.logger.debug(`Running ngcc on ${this.constructor.name}.`);
|
||||
|
||||
const {processingMetadataPerEntryPoint, tasks} = analyzeEntryPoints();
|
||||
|
@ -31,17 +29,8 @@ export class SingleProcessExecutor implements Executor {
|
|||
|
||||
// Process all tasks.
|
||||
for (const task of tasks) {
|
||||
const processingMeta = processingMetadataPerEntryPoint.get(task.entryPoint.path) !;
|
||||
|
||||
// If we only need one format processed and we already have one for the corresponding
|
||||
// entry-point, skip the task.
|
||||
if (!options.compileAllFormats && processingMeta.hasAnyProcessedFormat) continue;
|
||||
|
||||
compile(task);
|
||||
}
|
||||
|
||||
// Check for entry-points for which we could not process any format at all.
|
||||
checkForUnprocessedEntryPoints(processingMetadataPerEntryPoint, options.propertiesToConsider);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -14,34 +14,12 @@ import {PackageJsonUpdater} from '../writing/package_json_updater';
|
|||
import {EntryPointProcessingMetadata, Task, TaskProcessingOutcome} from './api';
|
||||
|
||||
|
||||
/**
|
||||
* A helper function for checking for unprocessed entry-points (i.e. entry-points for which we could
|
||||
* not process any format at all).
|
||||
*/
|
||||
export const checkForUnprocessedEntryPoints =
|
||||
(processingMetadataPerEntryPoint: Map<string, EntryPointProcessingMetadata>,
|
||||
propertiesToConsider: string[]): void => {
|
||||
const unprocessedEntryPointPaths =
|
||||
Array.from(processingMetadataPerEntryPoint.entries())
|
||||
.filter(([, processingMeta]) => !processingMeta.hasAnyProcessedFormat)
|
||||
.map(([entryPointPath]) => `\n - ${entryPointPath}`)
|
||||
.join('');
|
||||
|
||||
if (unprocessedEntryPointPaths) {
|
||||
throw new Error(
|
||||
'Failed to compile any formats for the following entry-points (tried ' +
|
||||
`${propertiesToConsider.join(', ')}): ${unprocessedEntryPointPaths}`);
|
||||
}
|
||||
};
|
||||
|
||||
/** A helper function for handling a task's being completed. */
|
||||
export const onTaskCompleted =
|
||||
(pkgJsonUpdater: PackageJsonUpdater,
|
||||
processingMetadataPerEntryPoint: Map<string, EntryPointProcessingMetadata>, task: Task,
|
||||
outcome: TaskProcessingOutcome, ): void => {
|
||||
const {entryPoint, formatPropertiesToMarkAsProcessed, processDts} = task;
|
||||
const processingMeta = processingMetadataPerEntryPoint.get(entryPoint.path) !;
|
||||
processingMeta.hasAnyProcessedFormat = true;
|
||||
|
||||
if (outcome === TaskProcessingOutcome.Processed) {
|
||||
const packageJsonPath = resolve(entryPoint.path, 'package.json');
|
||||
|
@ -49,6 +27,7 @@ export const onTaskCompleted =
|
|||
[...formatPropertiesToMarkAsProcessed];
|
||||
|
||||
if (processDts) {
|
||||
const processingMeta = processingMetadataPerEntryPoint.get(entryPoint.path) !;
|
||||
processingMeta.hasProcessedTypings = true;
|
||||
propsToMarkAsProcessed.push('typings');
|
||||
}
|
||||
|
|
|
@ -139,15 +139,25 @@ export function mainNgcc(
|
|||
targetEntryPointPath, pathMappings, supportedPropertiesToConsider, compileAllFormats);
|
||||
|
||||
const processingMetadataPerEntryPoint = new Map<string, EntryPointProcessingMetadata>();
|
||||
const unprocessableEntryPointPaths: string[] = [];
|
||||
const tasks: Task[] = [];
|
||||
|
||||
for (const entryPoint of entryPoints) {
|
||||
const packageJson = entryPoint.packageJson;
|
||||
const hasProcessedTypings = hasBeenProcessed(packageJson, 'typings', entryPoint.path);
|
||||
const {propertiesToProcess, equivalentPropertiesMap} =
|
||||
getPropertiesToProcess(packageJson, supportedPropertiesToConsider);
|
||||
getPropertiesToProcess(packageJson, supportedPropertiesToConsider, compileAllFormats);
|
||||
let processDts = !hasProcessedTypings;
|
||||
|
||||
if (propertiesToProcess.length === 0) {
|
||||
// This entry-point is unprocessable (i.e. there is no format property that is of interest
|
||||
// and can be processed). This will result in an error, but continue looping over
|
||||
// entry-points in order to collect all unprocessable ones and display a more informative
|
||||
// error.
|
||||
unprocessableEntryPointPaths.push(entryPoint.path);
|
||||
continue;
|
||||
}
|
||||
|
||||
for (const formatProperty of propertiesToProcess) {
|
||||
const formatPropertiesToMarkAsProcessed = equivalentPropertiesMap.get(formatProperty) !;
|
||||
tasks.push({entryPoint, formatProperty, formatPropertiesToMarkAsProcessed, processDts});
|
||||
|
@ -156,10 +166,15 @@ export function mainNgcc(
|
|||
processDts = false;
|
||||
}
|
||||
|
||||
processingMetadataPerEntryPoint.set(entryPoint.path, {
|
||||
hasProcessedTypings,
|
||||
hasAnyProcessedFormat: false,
|
||||
});
|
||||
processingMetadataPerEntryPoint.set(entryPoint.path, {hasProcessedTypings});
|
||||
}
|
||||
|
||||
// Check for entry-points for which we could not process any format at all.
|
||||
if (unprocessableEntryPointPaths.length > 0) {
|
||||
throw new Error(
|
||||
'Unable to process any formats for the following entry-points (tried ' +
|
||||
`${propertiesToConsider.join(', ')}): ` +
|
||||
unprocessableEntryPointPaths.map(path => `\n - ${path}`).join(''));
|
||||
}
|
||||
|
||||
return {processingMetadataPerEntryPoint, tasks};
|
||||
|
@ -220,9 +235,8 @@ export function mainNgcc(
|
|||
|
||||
// The executor for actually planning and getting the work done.
|
||||
const executor = getExecutor(async, logger, pkgJsonUpdater);
|
||||
const execOpts = {compileAllFormats, propertiesToConsider};
|
||||
|
||||
return executor.execute(analyzeEntryPoints, createCompileFn, execOpts);
|
||||
return executor.execute(analyzeEntryPoints, createCompileFn);
|
||||
}
|
||||
|
||||
function ensureSupportedProperties(properties: string[]): EntryPointJsonProperty[] {
|
||||
|
@ -376,7 +390,8 @@ function logInvalidEntryPoints(logger: Logger, invalidEntryPoints: InvalidEntryP
|
|||
* former has been processed.
|
||||
*/
|
||||
function getPropertiesToProcess(
|
||||
packageJson: EntryPointPackageJson, propertiesToConsider: EntryPointJsonProperty[]): {
|
||||
packageJson: EntryPointPackageJson, propertiesToConsider: EntryPointJsonProperty[],
|
||||
compileAllFormats: boolean): {
|
||||
propertiesToProcess: EntryPointJsonProperty[];
|
||||
equivalentPropertiesMap: Map<EntryPointJsonProperty, EntryPointJsonProperty[]>;
|
||||
} {
|
||||
|
@ -395,6 +410,9 @@ function getPropertiesToProcess(
|
|||
// Process this property, because it is the first one to map to this format-path.
|
||||
formatPathsToConsider.add(formatPath);
|
||||
propertiesToProcess.push(prop);
|
||||
|
||||
// If we only need one format processed, there is no need to process any more properties.
|
||||
if (!compileAllFormats) break;
|
||||
}
|
||||
|
||||
const formatPathToProperties: {[formatPath: string]: EntryPointJsonProperty[]} = {};
|
||||
|
|
|
@ -6,7 +6,7 @@
|
|||
* found in the LICENSE file at https://angular.io/license
|
||||
*/
|
||||
import {AbsoluteFsPath, FileSystem, absoluteFrom, getFileSystem, join} from '../../../src/ngtsc/file_system';
|
||||
import {Folder, MockFileSystem, runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
|
||||
import {Folder, MockFileSystem, TestFile, runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
|
||||
import {loadStandardTestFiles, loadTestFiles} from '../../../test/helpers';
|
||||
import {mainNgcc} from '../../src/main';
|
||||
import {markAsProcessed} from '../../src/packages/build_marker';
|
||||
|
@ -57,6 +57,36 @@ runInEachFileSystem(() => {
|
|||
});
|
||||
});
|
||||
|
||||
it('should throw, if some of the entry-points are unprocessable', () => {
|
||||
const createEntryPoint = (name: string, prop: EntryPointJsonProperty): TestFile[] => {
|
||||
return [
|
||||
{
|
||||
name: _(`/dist/${name}/package.json`),
|
||||
contents: `{"name": "${name}", "typings": "./index.d.ts", "${prop}": "./index.js"}`,
|
||||
},
|
||||
{name: _(`/dist/${name}/index.js`), contents: 'var DUMMY_DATA = true;'},
|
||||
{name: _(`/dist/${name}/index.d.ts`), contents: 'export type DummyData = boolean;'},
|
||||
{name: _(`/dist/${name}/index.metadata.json`), contents: 'DUMMY DATA'},
|
||||
];
|
||||
};
|
||||
|
||||
loadTestFiles([
|
||||
...createEntryPoint('processable-1', 'es2015'),
|
||||
...createEntryPoint('unprocessable-2', 'main'),
|
||||
...createEntryPoint('unprocessable-3', 'main'),
|
||||
]);
|
||||
|
||||
expect(() => mainNgcc({
|
||||
basePath: '/dist',
|
||||
propertiesToConsider: ['es2015', 'fesm5', 'module'],
|
||||
logger: new MockLogger(),
|
||||
}))
|
||||
.toThrowError(
|
||||
'Unable to process any formats for the following entry-points (tried es2015, fesm5, module): \n' +
|
||||
` - ${_('/dist/unprocessable-2')}\n` +
|
||||
` - ${_('/dist/unprocessable-3')}`);
|
||||
});
|
||||
|
||||
it('should throw, if an error happens during processing', () => {
|
||||
spyOn(Transformer.prototype, 'transform').and.throwError('Test error.');
|
||||
|
||||
|
@ -84,6 +114,40 @@ runInEachFileSystem(() => {
|
|||
await promise;
|
||||
});
|
||||
|
||||
it('should reject, if some of the entry-points are unprocessable', async() => {
|
||||
const createEntryPoint = (name: string, prop: EntryPointJsonProperty): TestFile[] => {
|
||||
return [
|
||||
{
|
||||
name: _(`/dist/${name}/package.json`),
|
||||
contents: `{"name": "${name}", "typings": "./index.d.ts", "${prop}": "./index.js"}`,
|
||||
},
|
||||
{name: _(`/dist/${name}/index.js`), contents: 'var DUMMY_DATA = true;'},
|
||||
{name: _(`/dist/${name}/index.d.ts`), contents: 'export type DummyData = boolean;'},
|
||||
{name: _(`/dist/${name}/index.metadata.json`), contents: 'DUMMY DATA'},
|
||||
];
|
||||
};
|
||||
|
||||
loadTestFiles([
|
||||
...createEntryPoint('processable-1', 'es2015'),
|
||||
...createEntryPoint('unprocessable-2', 'main'),
|
||||
...createEntryPoint('unprocessable-3', 'main'),
|
||||
]);
|
||||
|
||||
const promise = mainNgcc({
|
||||
basePath: '/dist',
|
||||
propertiesToConsider: ['es2015', 'fesm5', 'module'],
|
||||
logger: new MockLogger(),
|
||||
async: true,
|
||||
});
|
||||
|
||||
await promise.then(
|
||||
() => Promise.reject('Expected promise to be rejected.'),
|
||||
err => expect(err).toEqual(new Error(
|
||||
'Unable to process any formats for the following entry-points (tried es2015, fesm5, module): \n' +
|
||||
` - ${_('/dist/unprocessable-2')}\n` +
|
||||
` - ${_('/dist/unprocessable-3')}`)));
|
||||
});
|
||||
|
||||
it('should reject, if an error happens during processing', async() => {
|
||||
spyOn(Transformer.prototype, 'transform').and.throwError('Test error.');
|
||||
|
||||
|
|
Loading…
Reference in New Issue