Skip to content

Commit

Permalink
refactor(@angular-devkit/build-angular): ability to perform DCE but r…
Browse files Browse the repository at this point in the history
…etain symbol names

Previously, we enabled the `keepNames` esbuild when mangling was disabled, this caused dead code to be retained because of the transformations that esbuild did to the input.

Input
```js
class foo {}
```

Output
```js
var l = Object.defineProperty,
  a = (s, c) => l(s, "name", { value: c, configurable: !0 });
class foo {}
a(foo, "foo");
```

Previously we enabled the `keepNames` esbuild option when mangling was disabled, which is actually not needed to retain the name of symbols but is needed for SSR because Domino relies on the `name` property on functions and classes.

Closes angular#22354

(cherry picked from commit 2c9a33d)
  • Loading branch information
alan-agius4 committed Dec 17, 2021
1 parent 4c9d72c commit 39c95da
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,8 @@ export async function getCommonConfig(wco: WebpackConfigOptions): Promise<Config
define: buildOptions.aot ? GLOBAL_DEFS_FOR_TERSER_WITH_AOT : GLOBAL_DEFS_FOR_TERSER,
sourcemap: scriptsSourceMap,
target: scriptTarget,
keepNames: !allowMangle || isPlatformServer,
keepIdentifierNames: !allowMangle || isPlatformServer,
keepNames: isPlatformServer,
removeLicenses: buildOptions.extractLicenses,
advanced: buildOptions.buildOptimizer,
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { ScriptTarget } from 'typescript';
import type { Compiler, sources } from 'webpack';
import { maxWorkers } from '../../utils/environment-options';
import { EsbuildExecutor } from './esbuild-executor';
import type { OptimizeRequestOptions } from './javascript-optimizer-worker';

/**
* The maximum number of Workers that will be created to execute optimize tasks.
Expand All @@ -28,10 +29,10 @@ const PLUGIN_NAME = 'angular-javascript-optimizer';
export interface JavaScriptOptimizerOptions {
/**
* Enables advanced optimizations in the underlying JavaScript optimizers.
* This currently increases the `terser` passes to 3 and enables the `pure_getters`
* This currently increases the `terser` passes to 2 and enables the `pure_getters`
* option for `terser`.
*/
advanced: boolean;
advanced?: boolean;

/**
* An object record of string keys that will be replaced with their respective values when found
Expand All @@ -44,7 +45,7 @@ export interface JavaScriptOptimizerOptions {
* The output sourcemap will be a full sourcemap containing the merge of the input sourcemap and
* all intermediate sourcemaps.
*/
sourcemap: boolean;
sourcemap?: boolean;

/**
* The ECMAScript version that should be used when generating output code.
Expand All @@ -56,13 +57,22 @@ export interface JavaScriptOptimizerOptions {
/**
* Enables the retention of identifier names and ensures that function and class names are
* present in the output code.
*
* **Note**: in some cases symbols are still renamed to avoid collisions.
*/
keepIdentifierNames: boolean;

/**
* Enables the retention of original name of classes and functions.
*
* **Note**: this causes increase of bundle size as it causes dead-code elimination to not work fully.
*/
keepNames: boolean;

/**
* Enables the removal of all license comments from the output code.
*/
removeLicenses: boolean;
removeLicenses?: boolean;
}

/**
Expand All @@ -74,7 +84,7 @@ export interface JavaScriptOptimizerOptions {
* optimizations not yet implemented by `esbuild`.
*/
export class JavaScriptOptimizerPlugin {
constructor(public options: Partial<JavaScriptOptimizerOptions> = {}) {}
constructor(public options: JavaScriptOptimizerOptions) {}

apply(compiler: Compiler) {
const { OriginalSource, SourceMapSource } = compiler.webpack.sources;
Expand Down Expand Up @@ -142,22 +152,25 @@ export class JavaScriptOptimizerPlugin {
}
}

let target = 2017;
let target: OptimizeRequestOptions['target'] = 2017;
if (this.options.target) {
if (this.options.target <= ScriptTarget.ES5) {
target = 5;
} else if (this.options.target < ScriptTarget.ESNext) {
target = Number(ScriptTarget[this.options.target].slice(2));
target = Number(
ScriptTarget[this.options.target].slice(2),
) as OptimizeRequestOptions['target'];
} else {
target = 2020;
}
}

// Setup the options used by all worker tasks
const optimizeOptions = {
const optimizeOptions: OptimizeRequestOptions = {
sourcemap: this.options.sourcemap,
define,
keepNames: this.options.keepNames,
keepIdentifierNames: this.options.keepIdentifierNames,
target,
removeLicenses: this.options.removeLicenses,
advanced: this.options.advanced,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,50 +11,60 @@ import type { TransformFailure, TransformResult } from 'esbuild';
import { minify } from 'terser';
import { EsbuildExecutor } from './esbuild-executor';

/**
* The options to use when optimizing.
*/
export interface OptimizeRequestOptions {
/**
* Controls advanced optimizations.
* Currently these are only terser related:
* * terser compress passes are set to 2
* * terser pure_getters option is enabled
*/
advanced?: boolean;
/**
* Specifies the string tokens that should be replaced with a defined value.
*/
define?: Record<string, string>;
/**
* Controls whether class, function, and variable names should be left intact
* throughout the output code.
*/
keepIdentifierNames: boolean;

/**
* Controls whether to retain the original name of classes and functions.
*/
keepNames: boolean;
/**
* Controls whether license text is removed from the output code.
* Within the CLI, this option is linked to the license extraction functionality.
*/
removeLicenses?: boolean;
/**
* Controls whether source maps should be generated.
*/
sourcemap?: boolean;
/**
* Specifies the target ECMAScript version for the output code.
*/
target: 5 | 2015 | 2016 | 2017 | 2018 | 2019 | 2020;
/**
* Controls whether esbuild should only use the WASM-variant instead of trying to
* use the native variant. Some platforms may not support the native-variant and
* this option allows one support test to be conducted prior to all the workers starting.
*/
alwaysUseWasm: boolean;
}

/**
* A request to optimize JavaScript using the supplied options.
*/
interface OptimizeRequest {
/**
* The options to use when optimizing.
*/
options: {
/**
* Controls advanced optimizations.
* Currently these are only terser related:
* * terser compress passes are set to 2
* * terser pure_getters option is enabled
*/
advanced: boolean;
/**
* Specifies the string tokens that should be replaced with a defined value.
*/
define?: Record<string, string>;
/**
* Controls whether class, function, and variable names should be left intact
* throughout the output code.
*/
keepNames: boolean;
/**
* Controls whether license text is removed from the output code.
* Within the CLI, this option is linked to the license extraction functionality.
*/
removeLicenses: boolean;
/**
* Controls whether source maps should be generated.
*/
sourcemap: boolean;
/**
* Specifies the target ECMAScript version for the output code.
*/
target: 5 | 2015 | 2016 | 2017 | 2018 | 2019 | 2020;
/**
* Controls whether esbuild should only use the WASM-variant instead of trying to
* use the native variant. Some platforms may not support the native-variant and
* this option allows one support test to be conducted prior to all the workers starting.
*/
alwaysUseWasm: boolean;
};
options: OptimizeRequestOptions;

/**
* The JavaScript asset to optimize.
Expand Down Expand Up @@ -142,7 +152,7 @@ async function optimizeWithEsbuild(
let result: TransformResult;
try {
result = await esbuild.transform(content, {
minifyIdentifiers: !options.keepNames,
minifyIdentifiers: !options.keepIdentifierNames,
minifySyntax: true,
// NOTE: Disabling whitespace ensures unused pure annotations are kept
minifyWhitespace: false,
Expand All @@ -151,6 +161,10 @@ async function optimizeWithEsbuild(
sourcefile: name,
sourcemap: options.sourcemap && 'external',
define: options.define,
// This option should always be disabled for browser builds as we don't rely on `.name`
// and causes deadcode to be retained which makes `NG_BUILD_MANGLE` unusable to investigate tree-shaking issues.
// We enable `keepNames` only for server builds as Domino relies on `.name`.
// Once we no longer rely on Domino for SSR we should be able to remove this.
keepNames: options.keepNames,
target: `es${options.target}`,
});
Expand Down Expand Up @@ -193,9 +207,9 @@ async function optimizeWithEsbuild(
async function optimizeWithTerser(
name: string,
code: string,
sourcemaps: boolean,
sourcemaps: boolean | undefined,
target: OptimizeRequest['options']['target'],
advanced: boolean,
advanced: boolean | undefined,
): Promise<{ code: string; map?: object }> {
const result = await minify(
{ [name]: code },
Expand All @@ -207,6 +221,8 @@ async function optimizeWithTerser(
ecma: target,
// esbuild in the first pass is used to minify identifiers instead of mangle here
mangle: false,
// esbuild in the first pass is used to minify function names
keep_fnames: true,
format: {
// ASCII output is enabled here as well to prevent terser from converting back to UTF-8
ascii_only: true,
Expand Down

0 comments on commit 39c95da

Please sign in to comment.