From bd1de32b336f447cd51555e6ac2f9ee95c26e794 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Fri, 9 Aug 2019 15:01:57 +0300 Subject: [PATCH] refactor(ngcc): minor code clean-up following #32052 (#32427) This commit addresses the review feedback from #32052, which was merged before addressing the feedback there. PR Close #32427 --- .../compiler-cli/ngcc/src/execution/api.ts | 19 +-- packages/compiler-cli/ngcc/src/main.ts | 119 +++++++++--------- .../ngcc/src/packages/build_marker.ts | 6 +- .../ngcc/src/packages/configuration.ts | 6 +- .../ngcc/src/packages/entry_point.ts | 8 +- .../ngcc/src/packages/entry_point_bundle.ts | 3 +- .../ngcc/src/packages/ngcc_compiler_host.ts | 3 +- 7 files changed, 83 insertions(+), 81 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/execution/api.ts b/packages/compiler-cli/ngcc/src/execution/api.ts index 2a8cc1b0d2..11149101d1 100644 --- a/packages/compiler-cli/ngcc/src/execution/api.ts +++ b/packages/compiler-cli/ngcc/src/execution/api.ts @@ -9,23 +9,23 @@ import {EntryPoint, EntryPointJsonProperty} from '../packages/entry_point'; /** The type of the function that analyzes entry-points and creates the list of tasks. */ -export type AnalyzeFn = () => { +export type AnalyzeEntryPointsFn = () => { processingMetadataPerEntryPoint: Map; tasks: Task[]; }; -/** - * The type of the function that creates the `compile()` function, which in turn can be used to - * process tasks. - */ -export type CreateCompileFn = - (onTaskCompleted: (task: Task, outcome: TaskProcessingOutcome) => void) => (task: Task) => void; +/** The type of the function that can process/compile a task. */ +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; /** * The type of the function 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 type ExecuteFn = (analyzeFn: AnalyzeFn, createCompileFn: CreateCompileFn) => void; +export type ExecuteFn = + (analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn) => void; /** Represents metadata related to the processing of an entry-point. */ export interface EntryPointProcessingMetadata { @@ -64,6 +64,9 @@ export interface Task { processDts: boolean; } +/** A function to be called once a task has been processed. */ +export type TaskCompletedCallback = (task: Task, outcome: TaskProcessingOutcome) => void; + /** Represents the outcome of processing a `Task`. */ export const enum TaskProcessingOutcome { /** The target format property was already processed - didn't have to do anything. */ diff --git a/packages/compiler-cli/ngcc/src/main.ts b/packages/compiler-cli/ngcc/src/main.ts index 679bdfb8f3..54b093bab1 100644 --- a/packages/compiler-cli/ngcc/src/main.ts +++ b/packages/compiler-cli/ngcc/src/main.ts @@ -16,12 +16,12 @@ import {ModuleResolver} from './dependencies/module_resolver'; import {UmdDependencyHost} from './dependencies/umd_dependency_host'; import {DirectoryWalkerEntryPointFinder} from './entry_point_finder/directory_walker_entry_point_finder'; import {TargetedEntryPointFinder} from './entry_point_finder/targeted_entry_point_finder'; -import {AnalyzeFn, CreateCompileFn, EntryPointProcessingMetadata, ExecuteFn, Task, TaskProcessingOutcome} from './execution/api'; +import {AnalyzeEntryPointsFn, CreateCompileFn, EntryPointProcessingMetadata, ExecuteFn, Task, TaskProcessingOutcome} from './execution/api'; import {ConsoleLogger, LogLevel} from './logging/console_logger'; import {Logger} from './logging/logger'; import {hasBeenProcessed, markAsProcessed} from './packages/build_marker'; import {NgccConfiguration} from './packages/configuration'; -import {EntryPoint, EntryPointJsonProperty, EntryPointPackageJson, SUPPORTED_FORMAT_PROPERTIES, getEntryPointFormat} from './packages/entry_point'; +import {EntryPoint, EntryPointJsonProperty, EntryPointPackageJson, PackageJsonFormatProperties, SUPPORTED_FORMAT_PROPERTIES, getEntryPointFormat} from './packages/entry_point'; import {makeEntryPointBundle} from './packages/entry_point_bundle'; import {Transformer} from './packages/transformer'; import {PathMappings} from './utils'; @@ -87,7 +87,7 @@ export function mainNgcc( const fileSystem = getFileSystem(); // The function for performing the analysis. - const analyzeFn: AnalyzeFn = () => { + const analyzeEntryPoints: AnalyzeEntryPointsFn = () => { const supportedPropertiesToConsider = ensureSupportedProperties(propertiesToConsider); const moduleResolver = new ModuleResolver(fileSystem, pathMappings); @@ -113,13 +113,12 @@ export function mainNgcc( for (const entryPoint of entryPoints) { const packageJson = entryPoint.packageJson; const hasProcessedTypings = hasBeenProcessed(packageJson, 'typings', entryPoint.path); - const {propertiesToProcess, propertyToPropertiesToMarkAsProcessed} = - getPropertiesToProcessAndMarkAsProcessed(packageJson, supportedPropertiesToConsider); + const {propertiesToProcess, equivalentPropertiesMap} = + getPropertiesToProcess(packageJson, supportedPropertiesToConsider); let processDts = !hasProcessedTypings; for (const formatProperty of propertiesToProcess) { - const formatPropertiesToMarkAsProcessed = - propertyToPropertiesToMarkAsProcessed.get(formatProperty) !; + const formatPropertiesToMarkAsProcessed = equivalentPropertiesMap.get(formatProperty) !; tasks.push({entryPoint, formatProperty, formatPropertiesToMarkAsProcessed, processDts}); // Only process typings for the first property (if not already processed). @@ -189,54 +188,55 @@ export function mainNgcc( }; // The function for actually planning and getting the work done. - const executeFn: ExecuteFn = (analyzeFn: AnalyzeFn, createCompileFn: CreateCompileFn) => { - const {processingMetadataPerEntryPoint, tasks} = analyzeFn(); - const compile = createCompileFn((task, outcome) => { - const {entryPoint, formatPropertiesToMarkAsProcessed, processDts} = task; - const processingMeta = processingMetadataPerEntryPoint.get(entryPoint.path) !; - processingMeta.hasAnyProcessedFormat = true; + const execute: ExecuteFn = + (analyzeEntryPoints: AnalyzeEntryPointsFn, createCompileFn: CreateCompileFn) => { + const {processingMetadataPerEntryPoint, tasks} = analyzeEntryPoints(); + const compile = createCompileFn((task, outcome) => { + const {entryPoint, formatPropertiesToMarkAsProcessed, processDts} = task; + const processingMeta = processingMetadataPerEntryPoint.get(entryPoint.path) !; + processingMeta.hasAnyProcessedFormat = true; - if (outcome === TaskProcessingOutcome.Processed) { - const packageJsonPath = fileSystem.resolve(entryPoint.path, 'package.json'); - const propsToMarkAsProcessed: (EntryPointJsonProperty | 'typings')[] = - [...formatPropertiesToMarkAsProcessed]; + if (outcome === TaskProcessingOutcome.Processed) { + const packageJsonPath = fileSystem.resolve(entryPoint.path, 'package.json'); + const propsToMarkAsProcessed: PackageJsonFormatProperties[] = + [...formatPropertiesToMarkAsProcessed]; - if (processDts) { - processingMeta.hasProcessedTypings = true; - propsToMarkAsProcessed.push('typings'); + if (processDts) { + processingMeta.hasProcessedTypings = true; + propsToMarkAsProcessed.push('typings'); + } + + markAsProcessed( + fileSystem, entryPoint.packageJson, packageJsonPath, propsToMarkAsProcessed); + } + }); + + // 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 (!compileAllFormats && processingMeta.hasAnyProcessedFormat) continue; + + compile(task); } - markAsProcessed( - fileSystem, entryPoint.packageJson, packageJsonPath, propsToMarkAsProcessed); - } - }); + // Check for entry-points for which we could not process any format at all. + const unprocessedEntryPointPaths = + Array.from(processingMetadataPerEntryPoint.entries()) + .filter(([, processingMeta]) => !processingMeta.hasAnyProcessedFormat) + .map(([entryPointPath]) => `\n - ${entryPointPath}`) + .join(''); - // Process all tasks. - for (const task of tasks) { - const processingMeta = processingMetadataPerEntryPoint.get(task.entryPoint.path) !; + if (unprocessedEntryPointPaths) { + throw new Error( + 'Failed to compile any formats for the following entry-points (tried ' + + `${propertiesToConsider.join(', ')}): ${unprocessedEntryPointPaths}`); + } + }; - // If we only need one format processed and we already have one for the corresponding - // entry-point, skip the task. - if (!compileAllFormats && processingMeta.hasAnyProcessedFormat) continue; - - compile(task); - } - - // Check for entry-points for which we could not process any format at all. - 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}`); - } - }; - - return executeFn(analyzeFn, createCompileFn); + return execute(analyzeEntryPoints, createCompileFn); } function ensureSupportedProperties(properties: string[]): EntryPointJsonProperty[] { @@ -370,16 +370,16 @@ function logInvalidEntryPoints(logger: Logger, invalidEntryPoints: InvalidEntryP /** * This function computes and returns the following: * - `propertiesToProcess`: An (ordered) list of properties that exist and need to be processed, - * based on the specified `propertiesToConsider`, the properties in `package.json` and their + * based on the provided `propertiesToConsider`, the properties in `package.json` and their * corresponding format-paths. NOTE: Only one property per format-path needs to be processed. - * - `propertyToPropertiesToMarkAsProcessed`: A mapping from each property in `propertiesToProcess` - * to the list of other properties in `package.json` that need to be marked as processed as soon - * as of the former being processed. + * - `equivalentPropertiesMap`: A mapping from each property in `propertiesToProcess` to the list of + * other format properties in `package.json` that need to be marked as processed as soon as the + * former has been processed. */ -function getPropertiesToProcessAndMarkAsProcessed( +function getPropertiesToProcess( packageJson: EntryPointPackageJson, propertiesToConsider: EntryPointJsonProperty[]): { propertiesToProcess: EntryPointJsonProperty[]; - propertyToPropertiesToMarkAsProcessed: Map; + equivalentPropertiesMap: Map; } { const formatPathsToConsider = new Set(); @@ -413,13 +413,12 @@ function getPropertiesToProcessAndMarkAsProcessed( list.push(prop); } - const propertyToPropertiesToMarkAsProcessed = - new Map(); + const equivalentPropertiesMap = new Map(); for (const prop of propertiesToConsider) { const formatPath = packageJson[prop] !; - const propertiesToMarkAsProcessed = formatPathToProperties[formatPath]; - propertyToPropertiesToMarkAsProcessed.set(prop, propertiesToMarkAsProcessed); + const equivalentProperties = formatPathToProperties[formatPath]; + equivalentPropertiesMap.set(prop, equivalentProperties); } - return {propertiesToProcess, propertyToPropertiesToMarkAsProcessed}; + return {propertiesToProcess, equivalentPropertiesMap}; } diff --git a/packages/compiler-cli/ngcc/src/packages/build_marker.ts b/packages/compiler-cli/ngcc/src/packages/build_marker.ts index fef0d910eb..3703183d63 100644 --- a/packages/compiler-cli/ngcc/src/packages/build_marker.ts +++ b/packages/compiler-cli/ngcc/src/packages/build_marker.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ import {AbsoluteFsPath, FileSystem, dirname} from '../../../src/ngtsc/file_system'; -import {EntryPointJsonProperty, EntryPointPackageJson} from './entry_point'; +import {EntryPointPackageJson, PackageJsonFormatProperties} from './entry_point'; export const NGCC_VERSION = '0.0.0-PLACEHOLDER'; @@ -23,7 +23,7 @@ export const NGCC_VERSION = '0.0.0-PLACEHOLDER'; * @throws Error if the entry-point has already been processed with a different ngcc version. */ export function hasBeenProcessed( - packageJson: EntryPointPackageJson, format: EntryPointJsonProperty | 'typings', + packageJson: EntryPointPackageJson, format: PackageJsonFormatProperties, entryPointPath: AbsoluteFsPath): boolean { if (!packageJson.__processed_by_ivy_ngcc__) { return false; @@ -50,7 +50,7 @@ export function hasBeenProcessed( */ export function markAsProcessed( fs: FileSystem, packageJson: EntryPointPackageJson, packageJsonPath: AbsoluteFsPath, - properties: (EntryPointJsonProperty | 'typings')[]) { + properties: PackageJsonFormatProperties[]) { const processed = packageJson.__processed_by_ivy_ngcc__ || (packageJson.__processed_by_ivy_ngcc__ = {}); diff --git a/packages/compiler-cli/ngcc/src/packages/configuration.ts b/packages/compiler-cli/ngcc/src/packages/configuration.ts index 3ae30f5591..cf8fd8e1c6 100644 --- a/packages/compiler-cli/ngcc/src/packages/configuration.ts +++ b/packages/compiler-cli/ngcc/src/packages/configuration.ts @@ -7,7 +7,7 @@ */ import * as vm from 'vm'; import {AbsoluteFsPath, FileSystem, dirname, join, resolve} from '../../../src/ngtsc/file_system'; -import {PackageJsonFormatProperties} from './entry_point'; +import {PackageJsonFormatPropertiesMap} from './entry_point'; /** * The format of a project level configuration file. @@ -41,7 +41,7 @@ export interface NgccEntryPointConfig { * This property, if provided, holds values that will override equivalent properties in an * entry-point's package.json file. */ - override?: PackageJsonFormatProperties; + override?: PackageJsonFormatPropertiesMap; } const NGCC_CONFIG_FILENAME = 'ngcc.config.js'; @@ -121,4 +121,4 @@ export class NgccConfiguration { } return processedEntryPoints; } -} \ No newline at end of file +} diff --git a/packages/compiler-cli/ngcc/src/packages/entry_point.ts b/packages/compiler-cli/ngcc/src/packages/entry_point.ts index 8884a16804..4132f42992 100644 --- a/packages/compiler-cli/ngcc/src/packages/entry_point.ts +++ b/packages/compiler-cli/ngcc/src/packages/entry_point.ts @@ -38,7 +38,7 @@ export interface EntryPoint { compiledByAngular: boolean; } -export interface PackageJsonFormatProperties { +export interface PackageJsonFormatPropertiesMap { fesm2015?: string; fesm5?: string; es2015?: string; // if exists then it is actually FESM2015 @@ -50,16 +50,18 @@ export interface PackageJsonFormatProperties { typings?: string; // TypeScript .d.ts files } +export type PackageJsonFormatProperties = keyof PackageJsonFormatPropertiesMap; + /** * The properties that may be loaded from the `package.json` file. */ -export interface EntryPointPackageJson extends PackageJsonFormatProperties { +export interface EntryPointPackageJson extends PackageJsonFormatPropertiesMap { name: string; scripts?: Record; __processed_by_ivy_ngcc__?: Record; } -export type EntryPointJsonProperty = Exclude; +export type EntryPointJsonProperty = Exclude; // We need to keep the elements of this const and the `EntryPointJsonProperty` type in sync. export const SUPPORTED_FORMAT_PROPERTIES: EntryPointJsonProperty[] = ['fesm2015', 'fesm5', 'es2015', 'esm2015', 'esm5', 'main', 'module']; diff --git a/packages/compiler-cli/ngcc/src/packages/entry_point_bundle.ts b/packages/compiler-cli/ngcc/src/packages/entry_point_bundle.ts index 6f7a951366..dec8293a4e 100644 --- a/packages/compiler-cli/ngcc/src/packages/entry_point_bundle.ts +++ b/packages/compiler-cli/ngcc/src/packages/entry_point_bundle.ts @@ -6,8 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ import * as ts from 'typescript'; -import {AbsoluteFsPath, FileSystem, absoluteFrom} from '../../../src/ngtsc/file_system'; -import {NgtscCompilerHost} from '../../../src/ngtsc/file_system/src/compiler_host'; +import {AbsoluteFsPath, FileSystem, NgtscCompilerHost, absoluteFrom} from '../../../src/ngtsc/file_system'; import {PathMappings} from '../utils'; import {BundleProgram, makeBundleProgram} from './bundle_program'; import {EntryPoint, EntryPointFormat} from './entry_point'; diff --git a/packages/compiler-cli/ngcc/src/packages/ngcc_compiler_host.ts b/packages/compiler-cli/ngcc/src/packages/ngcc_compiler_host.ts index 161e34f7cc..8e52769814 100644 --- a/packages/compiler-cli/ngcc/src/packages/ngcc_compiler_host.ts +++ b/packages/compiler-cli/ngcc/src/packages/ngcc_compiler_host.ts @@ -7,8 +7,7 @@ */ import * as ts from 'typescript'; -import {FileSystem} from '../../../src/ngtsc/file_system'; -import {NgtscCompilerHost} from '../../../src/ngtsc/file_system/src/compiler_host'; +import {FileSystem, NgtscCompilerHost} from '../../../src/ngtsc/file_system'; import {isRelativePath} from '../utils'; /**