Tree-shake parameter defaults (replaces #4498) (#4510)

* Recreate parameter tree-shaking but keep boolean return values * Properly include parameters when tree-shaking is disabled Also improve side effect detection for unused parameter defaults * Make null checks more compact * Include super class parameter defaults * Include return value parameter defaults * Limit feature to top-level functions * Remove path from deoptimization signature * Ensure class methods are deoptimized * Add comment * Class fields need Node 12 * Improve coverage * Improve coverage
rollup · May 27, 2022 · 46a193b · 46a193b
1 parent e823ede
commit 46a193b
Show file tree

Hide file tree

Showing 126 changed files with 1,064 additions and 944 deletions.
diff --git a/rollup.config.ts b/rollup.config.ts
@@ -6,7 +6,7 @@ import commonjs from '@rollup/plugin-commonjs';
 import json from '@rollup/plugin-json';
 import { nodeResolve } from '@rollup/plugin-node-resolve';
 import typescript from '@rollup/plugin-typescript';
-import type { RollupOptions, WarningHandlerWithDefault } from 'rollup';
+import type { Plugin, RollupOptions, WarningHandlerWithDefault } from 'rollup';
 import { string } from 'rollup-plugin-string';
 import { terser } from 'rollup-plugin-terser';
 import addCliEntry from './build-plugins/add-cli-entry';
@@ -65,7 +65,7 @@ const treeshake = {
 	tryCatchDeoptimization: false
 };
 
-const nodePlugins = [
+const nodePlugins: Plugin[] = [
 	alias(moduleAliases),
 	nodeResolve(),
 	json(),

diff --git a/src/ast/nodes/ArrayExpression.ts b/src/ast/nodes/ArrayExpression.ts
@@ -2,7 +2,12 @@ import type { CallOptions } from '../CallOptions';
 import type { DeoptimizableEntity } from '../DeoptimizableEntity';
 import type { HasEffectsContext } from '../ExecutionContext';
 import type { NodeEvent } from '../NodeEvents';
-import { type ObjectPath, type PathTracker, UnknownInteger } from '../utils/PathTracker';
+import {
+	type ObjectPath,
+	type PathTracker,
+	UNKNOWN_PATH,
+	UnknownInteger
+} from '../utils/PathTracker';
 import { UNDEFINED_EXPRESSION, UNKNOWN_LITERAL_NUMBER } from '../values';
 import type * as NodeType from './NodeType';
 import SpreadElement from './SpreadElement';
@@ -14,6 +19,7 @@ import { ObjectEntity, type ObjectProperty } from './shared/ObjectEntity';
 export default class ArrayExpression extends NodeBase {
 	declare elements: readonly (ExpressionNode | SpreadElement | null)[];
 	declare type: NodeType.tArrayExpression;
+	protected deoptimized = false;
 	private objectEntity: ObjectEntity | null = null;
 
 	deoptimizePath(path: ObjectPath): void {
@@ -72,6 +78,21 @@ export default class ArrayExpression extends NodeBase {
 		return this.getObjectEntity().hasEffectsWhenCalledAtPath(path, callOptions, context);
 	}
 
+	protected applyDeoptimizations(): void {
+		this.deoptimized = true;
+		let hasSpread = false;
+		for (let index = 0; index < this.elements.length; index++) {
+			const element = this.elements[index];
+			if (hasSpread || element instanceof SpreadElement) {
+				if (element) {
+					hasSpread = true;
+					element.deoptimizePath(UNKNOWN_PATH);
+				}
+			}
+		}
+		this.context.requestTreeshakingPass();
+	}
+
 	private getObjectEntity(): ObjectEntity {
 		if (this.objectEntity !== null) {
 			return this.objectEntity;
@@ -82,7 +103,7 @@ export default class ArrayExpression extends NodeBase {
 		let hasSpread = false;
 		for (let index = 0; index < this.elements.length; index++) {
 			const element = this.elements[index];
-			if (element instanceof SpreadElement || hasSpread) {
+			if (hasSpread || element instanceof SpreadElement) {
 				if (element) {
 					hasSpread = true;
 					properties.unshift({ key: UnknownInteger, kind: 'init', property: element });

diff --git a/src/ast/nodes/ArrayPattern.ts b/src/ast/nodes/ArrayPattern.ts
@@ -16,9 +16,7 @@ export default class ArrayPattern extends NodeBase implements PatternNode {
 		exportNamesByVariable: ReadonlyMap<Variable, readonly string[]>
 	): void {
 		for (const element of this.elements) {
-			if (element !== null) {
-				element.addExportedVariables(variables, exportNamesByVariable);
-			}
+			element?.addExportedVariables(variables, exportNamesByVariable);
 		}
 	}
 
@@ -32,30 +30,24 @@ export default class ArrayPattern extends NodeBase implements PatternNode {
 		return variables;
 	}
 
-	deoptimizePath(path: ObjectPath): void {
-		if (path.length === 0) {
-			for (const element of this.elements) {
-				if (element !== null) {
-					element.deoptimizePath(path);
-				}
-			}
+	// Patterns can only be deoptimized at the empty path at the moment
+	deoptimizePath(): void {
+		for (const element of this.elements) {
+			element?.deoptimizePath(EMPTY_PATH);
 		}
 	}
 
-	hasEffectsWhenAssignedAtPath(path: ObjectPath, context: HasEffectsContext): boolean {
-		if (path.length > 0) return true;
+	// Patterns are only checked at the emtpy path at the moment
+	hasEffectsWhenAssignedAtPath(_path: ObjectPath, context: HasEffectsContext): boolean {
 		for (const element of this.elements) {
-			if (element !== null && element.hasEffectsWhenAssignedAtPath(EMPTY_PATH, context))
-				return true;
+			if (element?.hasEffectsWhenAssignedAtPath(EMPTY_PATH, context)) return true;
 		}
 		return false;
 	}
 
 	markDeclarationReached(): void {
 		for (const element of this.elements) {
-			if (element !== null) {
-				element.markDeclarationReached();
-			}
+			element?.markDeclarationReached();
 		}
 	}
 }
diff --git a/src/ast/nodes/ArrowFunctionExpression.ts b/src/ast/nodes/ArrowFunctionExpression.ts
@@ -1,13 +1,12 @@
 import { type CallOptions } from '../CallOptions';
-import { type HasEffectsContext, InclusionContext } from '../ExecutionContext';
+import { type HasEffectsContext } from '../ExecutionContext';
 import ReturnValueScope from '../scopes/ReturnValueScope';
 import type Scope from '../scopes/Scope';
 import { type ObjectPath } from '../utils/PathTracker';
 import BlockStatement from './BlockStatement';
-import Identifier from './Identifier';
 import * as NodeType from './NodeType';
 import FunctionBase from './shared/FunctionBase';
-import { type ExpressionNode, IncludeChildren } from './shared/Node';
+import { type ExpressionNode } from './shared/Node';
 import { ObjectEntity } from './shared/ObjectEntity';
 import { OBJECT_PROTOTYPE } from './shared/ObjectPrototype';
 import type { PatternNode } from './shared/Pattern';
@@ -48,15 +47,6 @@ export default class ArrowFunctionExpression extends FunctionBase {
 		return false;
 	}
 
-	include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
-		super.include(context, includeChildrenRecursively);
-		for (const param of this.params) {
-			if (!(param instanceof Identifier)) {
-				param.include(context, includeChildrenRecursively);
-			}
-		}
-	}
-
 	protected getObjectEntity(): ObjectEntity {
 		if (this.objectEntity !== null) {
 			return this.objectEntity;

diff --git a/src/ast/nodes/AssignmentPattern.ts b/src/ast/nodes/AssignmentPattern.ts
@@ -2,12 +2,13 @@ import type MagicString from 'magic-string';
 import { BLANK } from '../../utils/blank';
 import type { NodeRenderOptions, RenderOptions } from '../../utils/renderHelpers';
 import type { HasEffectsContext } from '../ExecutionContext';
+import { InclusionContext } from '../ExecutionContext';
 import { EMPTY_PATH, type ObjectPath, UNKNOWN_PATH } from '../utils/PathTracker';
 import type LocalVariable from '../variables/LocalVariable';
 import type Variable from '../variables/Variable';
 import type * as NodeType from './NodeType';
 import type { ExpressionEntity } from './shared/Expression';
-import { type ExpressionNode, NodeBase } from './shared/Node';
+import { type ExpressionNode, IncludeChildren, NodeBase } from './shared/Node';
 import type { PatternNode } from './shared/Pattern';
 
 export default class AssignmentPattern extends NodeBase implements PatternNode {
@@ -35,6 +36,15 @@ export default class AssignmentPattern extends NodeBase implements PatternNode {
 		return path.length > 0 || this.left.hasEffectsWhenAssignedAtPath(EMPTY_PATH, context);
 	}
 
+	// Note that FunctionBase may directly include .left and .right without
+	// including the pattern itself. This is how default parameter tree-shaking
+	// works at the moment.
+	include(context: InclusionContext, includeChildrenRecursively: IncludeChildren): void {
+		this.included = true;
+		this.left.include(context, includeChildrenRecursively);
+		this.right.include(context, includeChildrenRecursively);
+	}
+
 	markDeclarationReached(): void {
 		this.left.markDeclarationReached();
 	}
@@ -45,7 +55,11 @@ export default class AssignmentPattern extends NodeBase implements PatternNode {
 		{ isShorthandProperty }: NodeRenderOptions = BLANK
 	): void {
 		this.left.render(code, options, { isShorthandProperty });
-		this.right.render(code, options);
+		if (this.right.included) {
+			this.right.render(code, options);
+		} else {
+			code.remove(this.left.end, this.end);
+		}
 	}
 
 	protected applyDeoptimizations(): void {

diff --git a/src/ast/nodes/AwaitExpression.ts b/src/ast/nodes/AwaitExpression.ts
@@ -1,5 +1,4 @@
 import type { InclusionContext } from '../ExecutionContext';
-import { UNKNOWN_PATH } from '../utils/PathTracker';
 import ArrowFunctionExpression from './ArrowFunctionExpression';
 import type * as NodeType from './NodeType';
 import FunctionNode from './shared/FunctionNode';
@@ -30,10 +29,4 @@ export default class AwaitExpression extends NodeBase {
 		}
 		this.argument.include(context, includeChildrenRecursively);
 	}
-
-	protected applyDeoptimizations(): void {
-		this.deoptimized = true;
-		this.argument.deoptimizePath(UNKNOWN_PATH);
-		this.context.requestTreeshakingPass();
-	}
 }
diff --git a/src/ast/nodes/BinaryExpression.ts b/src/ast/nodes/BinaryExpression.ts
@@ -40,9 +40,10 @@ const binaryOperators: {
 	'>>': (left: any, right: any) => left >> right,
 	'>>>': (left: any, right: any) => left >>> right,
 	'^': (left: any, right: any) => left ^ right,
-	in: () => UnknownValue,
-	instanceof: () => UnknownValue,
 	'|': (left: any, right: any) => left | right
+	// We use the fallback for cases where we return something unknown
+	// in: () => UnknownValue,
+	// instanceof: () => UnknownValue,
 };
 
 export default class BinaryExpression extends NodeBase implements DeoptimizableEntity {
@@ -60,10 +61,10 @@ export default class BinaryExpression extends NodeBase implements DeoptimizableE
 	): LiteralValueOrUnknown {
 		if (path.length > 0) return UnknownValue;
 		const leftValue = this.left.getLiteralValueAtPath(EMPTY_PATH, recursionTracker, origin);
-		if (leftValue === UnknownValue) return UnknownValue;
+		if (typeof leftValue === 'symbol') return UnknownValue;
 
 		const rightValue = this.right.getLiteralValueAtPath(EMPTY_PATH, recursionTracker, origin);
-		if (rightValue === UnknownValue) return UnknownValue;
+		if (typeof rightValue === 'symbol') return UnknownValue;
 
 		const operatorFn = binaryOperators[this.operator];
 		if (!operatorFn) return UnknownValue;