From b46b3cf43ef7df5a2a3ff31dd6c8afd35e92fff2 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Sat, 25 Sep 2021 17:26:04 +0200 Subject: [PATCH] refactor: remove remaining dynamic require usages in package output (#43431) Removes the remaining usages of dynamic require statements in the package output. Since we declare all shipped packages as strict ESM, we cannot use dynamic require statements anymore. This commit switches these usages to actual `import` statements. Note: Tsickle continues to remain an optional dependency since bundling does not work with its UMD package output. Also tsickle is rarely used by consumers, if at all, so bundling does not really provide any significant value. To continue keeping tsickle optional (since it's still needed by the `annotateForClosureCompiler` option which is also respected in ngtsc), we pass-through a tsickle instance as a parameter to `main`. This allows us to keep the compile functions synchronous without having to refactor the majority of the watch compilation code, and majority of tests for ngc, ngtsc. Consumers (like the `ngc` bin entry-point) can then load tsickle based on their module format. e.g. tsickle can be imported through `require` to keep everything sync, but in ESM, the dynamic import can be used beforehand to pass `tsickle` to the `main` function. We can revisit this in the future but for now this does the trick without exceeding the scope of this commit.. PR Close #43431 --- packages/compiler-cli/BUILD.bazel | 2 + .../src/dependencies/dependency_resolver.ts | 3 +- packages/compiler-cli/src/bin/ngc.ts | 28 +++++++++++--- packages/compiler-cli/src/extract_i18n.ts | 8 ++-- packages/compiler-cli/src/main.ts | 38 +++++++++++-------- .../src/transformers/compiler_host.ts | 4 +- packages/compiler-cli/test/BUILD.bazel | 2 +- packages/compiler-cli/test/ngc_spec.ts | 23 +++++++---- packages/compiler-cli/test/ngtsc/BUILD.bazel | 1 + packages/compiler-cli/test/ngtsc/env.ts | 6 +-- packages/compiler/test/aot/compiler_spec.ts | 11 +++--- packages/compiler/testing/BUILD.bazel | 2 + .../testing/src/output/source_map_util.ts | 10 +++-- 13 files changed, 91 insertions(+), 47 deletions(-) diff --git a/packages/compiler-cli/BUILD.bazel b/packages/compiler-cli/BUILD.bazel index ee603471958..c0d3110687f 100644 --- a/packages/compiler-cli/BUILD.bazel +++ b/packages/compiler-cli/BUILD.bazel @@ -41,6 +41,7 @@ esbuild( external = [ "@angular/compiler", "typescript", + "tsickle", "@babel/core", "reflect-metadata", "minimist", @@ -96,6 +97,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/typecheck", "//packages/compiler-cli/src/ngtsc/typecheck/api", "@npm//@bazel/typescript", + "@npm//@types/minimist", "@npm//@types/node", "@npm//chokidar", "@npm//minimist", diff --git a/packages/compiler-cli/ngcc/src/dependencies/dependency_resolver.ts b/packages/compiler-cli/ngcc/src/dependencies/dependency_resolver.ts index 7d6d7f3219a..69754de8ae3 100644 --- a/packages/compiler-cli/ngcc/src/dependencies/dependency_resolver.ts +++ b/packages/compiler-cli/ngcc/src/dependencies/dependency_resolver.ts @@ -7,6 +7,7 @@ */ import {DepGraph} from 'dependency-graph'; +import module from 'module'; import {AbsoluteFsPath, ReadonlyFileSystem} from '../../../src/ngtsc/file_system'; import {Logger} from '../../../src/ngtsc/logging'; @@ -16,7 +17,7 @@ import {PartiallyOrderedList} from '../utils'; import {createDependencyInfo, DependencyHost, EntryPointWithDependencies} from './dependency_host'; -const builtinNodeJsModules = new Set(require('module').builtinModules); +const builtinNodeJsModules = new Set(module.builtinModules); /** * Holds information about entry points that are removed because diff --git a/packages/compiler-cli/src/bin/ngc.ts b/packages/compiler-cli/src/bin/ngc.ts index 103aa26509b..6cc2abbf178 100644 --- a/packages/compiler-cli/src/bin/ngc.ts +++ b/packages/compiler-cli/src/bin/ngc.ts @@ -13,8 +13,26 @@ import 'reflect-metadata'; import {NodeJSFileSystem, setFileSystem} from '../ngtsc/file_system'; import {main} from '../main'; -process.title = 'Angular Compiler (ngc)'; -const args = process.argv.slice(2); -// We are running the real compiler so run against the real file-system -setFileSystem(new NodeJSFileSystem()); -process.exitCode = main(args); +async function runNgcComamnd() { + process.title = 'Angular Compiler (ngc)'; + const args = process.argv.slice(2); + // We are running the real compiler so run against the real file-system + setFileSystem(new NodeJSFileSystem()); + + let tsickleModule: typeof import('tsickle')|undefined; + + // Load tsickle if it's available. We load it here because tsickle + // is not needed in all Angular projects directly using `ngc`. + try { + tsickleModule = (await import('tsickle')).default; + } catch { + } + + process.exitCode = + main(args, undefined, undefined, undefined, undefined, undefined, tsickleModule); +} + +runNgcComamnd().catch(e => { + console.error(e); + process.exitCode = 1; +}); \ No newline at end of file diff --git a/packages/compiler-cli/src/extract_i18n.ts b/packages/compiler-cli/src/extract_i18n.ts index 8e8d7f89118..47c22bfe118 100644 --- a/packages/compiler-cli/src/extract_i18n.ts +++ b/packages/compiler-cli/src/extract_i18n.ts @@ -10,9 +10,11 @@ * Extract i18n messages from source code */ -import * as api from './transformers/api'; -import {ParsedConfiguration} from './perform_compile'; +import minimist from 'minimist'; + import {main, readCommandLineAndConfiguration} from './main'; +import {ParsedConfiguration} from './perform_compile'; +import * as api from './transformers/api'; export function mainXi18n( args: string[], consoleError: (msg: string) => void = console.error): number { @@ -22,7 +24,7 @@ export function mainXi18n( function readXi18nCommandLineAndConfiguration(args: string[]): ParsedConfiguration { const options: api.CompilerOptions = {}; - const parsedArgs = require('minimist')(args); + const parsedArgs = minimist(args); if (parsedArgs.outFile) options.i18nOutFile = parsedArgs.outFile; if (parsedArgs.i18nFormat) options.i18nOutFormat = parsedArgs.i18nFormat; if (parsedArgs.locale) options.i18nOutLocale = parsedArgs.locale; diff --git a/packages/compiler-cli/src/main.ts b/packages/compiler-cli/src/main.ts index c00cb1281ee..9aad87956b8 100644 --- a/packages/compiler-cli/src/main.ts +++ b/packages/compiler-cli/src/main.ts @@ -6,20 +6,23 @@ * found in the LICENSE file at https://angular.io/license */ -import * as tsickle from 'tsickle'; +import minimist from 'minimist'; import ts from 'typescript'; +import type {TsickleHost} from 'tsickle'; import {Diagnostics, exitCodeFromResult, filterErrorsAndWarnings, formatDiagnostics, ParsedConfiguration, performCompilation, readConfiguration} from './perform_compile'; import {createPerformWatchHost, performWatchCompilation} from './perform_watch'; import * as api from './transformers/api'; import {GENERATED_FILES} from './transformers/util'; +type TsickleModule = typeof import('tsickle'); + export function main( args: string[], consoleError: (s: string) => void = console.error, config?: NgcParsedConfiguration, customTransformers?: api.CustomTransformers, programReuse?: { program: api.Program|undefined, }, - modifiedResourceFiles?: Set|null): number { + modifiedResourceFiles?: Set|null, tsickle?: TsickleModule): number { let {project, rootNames, options, errors: configErrors, watch, emitFlags} = config || readNgcCommandLineAndConfiguration(args); if (configErrors.length) { @@ -40,7 +43,7 @@ export function main( options, emitFlags, oldProgram, - emitCallback: createEmitCallback(options), + emitCallback: createEmitCallback(options, tsickle), customTransformers, modifiedResourceFiles }); @@ -52,8 +55,8 @@ export function main( export function mainDiagnosticsForTest( args: string[], config?: NgcParsedConfiguration, - programReuse?: {program: api.Program|undefined}, - modifiedResourceFiles?: Set|null): ReadonlyArray { + programReuse?: {program: api.Program|undefined}, modifiedResourceFiles?: Set|null, + tsickle?: TsickleModule): ReadonlyArray { let {project, rootNames, options, errors: configErrors, watch, emitFlags} = config || readNgcCommandLineAndConfiguration(args); if (configErrors.length) { @@ -71,7 +74,7 @@ export function mainDiagnosticsForTest( emitFlags, oldProgram, modifiedResourceFiles, - emitCallback: createEmitCallback(options), + emitCallback: createEmitCallback(options, tsickle), }); if (programReuse !== undefined) { @@ -81,12 +84,16 @@ export function mainDiagnosticsForTest( return compileDiags; } -function createEmitCallback(options: api.CompilerOptions): api.TsEmitCallback|undefined { +function createEmitCallback( + options: api.CompilerOptions, tsickle?: TsickleModule): api.TsEmitCallback|undefined { if (!options.annotateForClosureCompiler) { return undefined; } + if (tsickle == undefined) { + throw Error('Tsickle is not provided but `annotateForClosureCompiler` is enabled.') + } const tsickleHost: Pick< - tsickle.TsickleHost, + TsickleHost, 'shouldSkipTsickleProcessing'|'pathToModuleName'|'shouldIgnoreWarningsForPath'| 'fileNameToModuleId'|'googmodule'|'untyped'|'convertIndexImportShorthand'| 'transformDecorators'|'transformTypesToClosure'> = { @@ -115,13 +122,12 @@ function createEmitCallback(options: api.CompilerOptions): api.TsEmitCallback|un host, options }) => - // tslint:disable-next-line:no-require-imports only depend on tsickle if requested - require('tsickle').emitWithTsickle( - program, {...tsickleHost, options, host, moduleResolutionHost: host}, host, options, - targetSourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, { - beforeTs: customTransformers.before, - afterTs: customTransformers.after, - }); + tsickle.emitWithTsickle( + program, {...tsickleHost, options, moduleResolutionHost: host}, host, options, + targetSourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, { + beforeTs: customTransformers.before, + afterTs: customTransformers.after, + }); } export interface NgcParsedConfiguration extends ParsedConfiguration { @@ -130,7 +136,7 @@ export interface NgcParsedConfiguration extends ParsedConfiguration { export function readNgcCommandLineAndConfiguration(args: string[]): NgcParsedConfiguration { const options: api.CompilerOptions = {}; - const parsedArgs = require('minimist')(args); + const parsedArgs = minimist(args); if (parsedArgs.i18nFile) options.i18nInFile = parsedArgs.i18nFile; if (parsedArgs.i18nFormat) options.i18nInFormat = parsedArgs.i18nFormat; if (parsedArgs.locale) options.i18nInLocale = parsedArgs.locale; diff --git a/packages/compiler-cli/src/transformers/compiler_host.ts b/packages/compiler-cli/src/transformers/compiler_host.ts index fa17484f5ec..4e180560a25 100644 --- a/packages/compiler-cli/src/transformers/compiler_host.ts +++ b/packages/compiler-cli/src/transformers/compiler_host.ts @@ -7,6 +7,7 @@ */ import {AotCompilerHost, collectExternalReferences, EmitterVisitorContext, GeneratedFile, ParseSourceSpan, syntaxError, TypeScriptEmitter} from '@angular/compiler'; +import fs from 'fs'; import * as path from 'path'; import ts from 'typescript'; @@ -253,7 +254,8 @@ export class TsCompilerAotCompilerTypeCheckHostAdapter implements ts.CompilerHos try { const modulePath = importedFile.substring(0, importedFile.length - moduleName.length) + importedFilePackageName; - const packageJson = require(modulePath + '/package.json'); + const packageJson = + JSON.parse(fs.readFileSync(modulePath + '/package.json', 'utf8')) as any; const packageTypings = join(modulePath, packageJson.typings); if (packageTypings === originalImportedFile) { moduleName = importedFilePackageName; diff --git a/packages/compiler-cli/test/BUILD.bazel b/packages/compiler-cli/test/BUILD.bazel index c2a8dd13acb..a8d295f693e 100644 --- a/packages/compiler-cli/test/BUILD.bazel +++ b/packages/compiler-cli/test/BUILD.bazel @@ -68,6 +68,7 @@ ts_library( ":test_utils", "//packages/compiler", "//packages/compiler-cli", + "@npm//tsickle", "@npm//typescript", ], ) @@ -92,7 +93,6 @@ jasmine_node_test( "//packages/core", "@npm//minimist", "@npm//rxjs", - "@npm//tsickle", ], ) diff --git a/packages/compiler-cli/test/ngc_spec.ts b/packages/compiler-cli/test/ngc_spec.ts index d81d9580970..f985679a6d0 100644 --- a/packages/compiler-cli/test/ngc_spec.ts +++ b/packages/compiler-cli/test/ngc_spec.ts @@ -8,9 +8,11 @@ import * as fs from 'fs'; import * as path from 'path'; +import * as tsickle from 'tsickle'; import ts from 'typescript'; import {main, mainDiagnosticsForTest, readCommandLineAndConfiguration, watchMode} from '../src/main'; + import {setup, stripAnsi} from './test_support'; describe('ngc transformer command-line', () => { @@ -676,7 +678,8 @@ describe('ngc transformer command-line', () => { export class MyModule {} `); - const exitCode = main(['-p', basePath], errorSpy); + const exitCode = + main(['-p', basePath], errorSpy, undefined, undefined, undefined, undefined, tsickle); expect(exitCode).toEqual(0); const mymodulejs = path.resolve(outDir, 'mymodule.js'); @@ -708,7 +711,8 @@ describe('ngc transformer command-line', () => { export class MyModule {} `); - const exitCode = main(['-p', basePath], errorSpy); + const exitCode = + main(['-p', basePath], errorSpy, undefined, undefined, undefined, undefined, tsickle); expect(exitCode).toEqual(0); const mymodulejs = path.resolve(outDir, 'mymodule.js'); @@ -749,7 +753,8 @@ describe('ngc transformer command-line', () => { export class MyModule {} `); - const exitCode = main(['-p', basePath], errorSpy); + const exitCode = + main(['-p', basePath], errorSpy, undefined, undefined, undefined, undefined, tsickle); expect(exitCode).toEqual(0); const mymodulejs = path.resolve(outDir, 'mymodule.js'); const mymoduleSource = fs.readFileSync(mymodulejs, 'utf8'); @@ -2118,10 +2123,12 @@ describe('ngc transformer command-line', () => { }); describe('tree shakeable services', () => { - function compileService(source: string): string { + function compileService(source: string, withTsickle = false): string { write('service.ts', source); - const exitCode = main(['-p', path.join(basePath, 'tsconfig.json')], errorSpy); + const exitCode = main( + ['-p', path.join(basePath, 'tsconfig.json')], errorSpy, undefined, undefined, undefined, + undefined, withTsickle ? tsickle : undefined); expect(exitCode).toEqual(0); const servicePath = path.resolve(outDir, 'service.js'); @@ -2203,7 +2210,7 @@ describe('ngc transformer command-line', () => { }, "files": ["service.ts"] }`); - const source = compileService(` + const input = ` import {Injectable} from '@angular/core'; import {Module} from './module'; @@ -2211,7 +2218,9 @@ describe('ngc transformer command-line', () => { providedIn: Module, }) export class Service {} - `); + `; + + const source = compileService(input, /* withTsickle */ true); expect(source).toMatch(/\/\*\* @nocollapse \*\/ Service\.ɵprov =/); }); diff --git a/packages/compiler-cli/test/ngtsc/BUILD.bazel b/packages/compiler-cli/test/ngtsc/BUILD.bazel index 3e3f209e70e..4d039b9223e 100644 --- a/packages/compiler-cli/test/ngtsc/BUILD.bazel +++ b/packages/compiler-cli/test/ngtsc/BUILD.bazel @@ -16,6 +16,7 @@ ts_library( "//packages/compiler-cli/src/ngtsc/util", "//packages/compiler-cli/test:test_utils", "@npm//source-map", + "@npm//tsickle", "@npm//typescript", ], ) diff --git a/packages/compiler-cli/test/ngtsc/env.ts b/packages/compiler-cli/test/ngtsc/env.ts index 150b532717b..165e04c16b9 100644 --- a/packages/compiler-cli/test/ngtsc/env.ts +++ b/packages/compiler-cli/test/ngtsc/env.ts @@ -8,6 +8,7 @@ import {CustomTransformers, defaultGatherDiagnostics, Program} from '@angular/compiler-cli'; import * as api from '@angular/compiler-cli/src/transformers/api'; +import * as tsickle from 'tsickle'; import ts from 'typescript'; import {createCompilerHost, createProgram} from '../../index'; @@ -21,7 +22,6 @@ import {DeclarationNode} from '../../src/ngtsc/reflection'; import {NgtscTestCompilerHost} from '../../src/ngtsc/testing'; import {setWrapHostForTest} from '../../src/transformers/compiler_host'; - /** * Manages a temporary testing directory structure and environment for testing ngtsc by feeding it * TypeScript code. @@ -220,7 +220,7 @@ export class NgtscTestEnvironment { } const exitCode = main( this.commandLineArgs, errorSpy, undefined, customTransformers, reuseProgram, - this.changedResources); + this.changedResources, tsickle); expect(errorSpy).not.toHaveBeenCalled(); expect(exitCode).toBe(0); if (this.multiCompileHostExt !== null) { @@ -241,7 +241,7 @@ export class NgtscTestEnvironment { } const diags = mainDiagnosticsForTest( - this.commandLineArgs, undefined, reuseProgram, this.changedResources); + this.commandLineArgs, undefined, reuseProgram, this.changedResources, tsickle); if (this.multiCompileHostExt !== null) { diff --git a/packages/compiler/test/aot/compiler_spec.ts b/packages/compiler/test/aot/compiler_spec.ts index d034356e8ae..0678b52eda1 100644 --- a/packages/compiler/test/aot/compiler_spec.ts +++ b/packages/compiler/test/aot/compiler_spec.ts @@ -56,11 +56,10 @@ describe('compiler (unbundled Angular)', () => { genFile => genFile.srcFileUrl === componentPath && genFile.genFileUrl.endsWith('.ts'))!; } - function findLineAndColumn( - file: string, token: string): {line: number|null, column: number|null} { + function findLineAndColumn(file: string, token: string): {line: number, column: number}|null { const index = file.indexOf(token); if (index === -1) { - return {line: null, column: null}; + return null; } const linesUntilToken = file.slice(0, index).split('\n'); const line = linesUntilToken.length; @@ -149,7 +148,7 @@ describe('compiler (unbundled Angular)', () => { const genFile = compileApp(); const genSource = toTypeScript(genFile); const sourceMap = extractSourceMap(genSource)!; - expect(originalPositionFor(sourceMap, findLineAndColumn(genSource, `'span'`))) + expect(originalPositionFor(sourceMap, findLineAndColumn(genSource, `'span'`)!)) .toEqual({line: 2, column: 3, source: ngUrl}); }); @@ -161,7 +160,7 @@ describe('compiler (unbundled Angular)', () => { const genFile = compileApp(); const genSource = toTypeScript(genFile); const sourceMap = extractSourceMap(genSource)!; - expect(originalPositionFor(sourceMap, findLineAndColumn(genSource, `someMethod()`))) + expect(originalPositionFor(sourceMap, findLineAndColumn(genSource, `someMethod()`)!)) .toEqual({line: 2, column: 9, source: ngUrl}); }); @@ -173,7 +172,7 @@ describe('compiler (unbundled Angular)', () => { const genFile = compileApp(); const genSource = toTypeScript(genFile); const sourceMap = extractSourceMap(genSource)!; - expect(originalPositionFor(sourceMap, findLineAndColumn(genSource, `someMethod()`))) + expect(originalPositionFor(sourceMap, findLineAndColumn(genSource, `someMethod()`)!)) .toEqual({line: 2, column: 9, source: ngUrl}); }); diff --git a/packages/compiler/testing/BUILD.bazel b/packages/compiler/testing/BUILD.bazel index 52bc2664cd1..0c8acaba463 100644 --- a/packages/compiler/testing/BUILD.bazel +++ b/packages/compiler/testing/BUILD.bazel @@ -17,5 +17,7 @@ ng_module( "//packages/compiler", "//packages/core", "@npm//@types/node", + "@npm//base64-js", + "@npm//source-map", ], ) diff --git a/packages/compiler/testing/src/output/source_map_util.ts b/packages/compiler/testing/src/output/source_map_util.ts index 525db6b0c18..34c80e31834 100644 --- a/packages/compiler/testing/src/output/source_map_util.ts +++ b/packages/compiler/testing/src/output/source_map_util.ts @@ -7,8 +7,8 @@ */ import {SourceMap} from '@angular/compiler'; -const b64 = require('base64-js'); -const SourceMapConsumer = require('source-map').SourceMapConsumer; +import b64 from 'base64-js'; +import {SourceMapConsumer} from 'source-map'; export interface SourceLocation { line: number; @@ -17,8 +17,10 @@ export interface SourceLocation { } export function originalPositionFor( - sourceMap: SourceMap, genPosition: {line: number|null, column: number|null}): SourceLocation { - const smc = new SourceMapConsumer(sourceMap); + sourceMap: SourceMap, genPosition: {line: number, column: number}): SourceLocation { + // Note: The `SourceMap` type from the compiler is different to `RawSourceMap` + // from the `source-map` package, but the method we rely on works as expected. + const smc = new SourceMapConsumer(sourceMap as any); // Note: We don't return the original object as it also contains a `name` property // which is always null and we don't want to include that in our assertions... const {line, column, source} = smc.originalPositionFor(genPosition);