* resolve #1711

* Severely limit the object path depth that is tree-shaken * Simplify the computed expression logic to avoid excessive assignments
rollup · Nov 11, 2017 · 9fadd5b · 9fadd5b
1 parent bc928fc
commit 9fadd5b
Show file tree

Hide file tree

Showing 10 changed files with 29 additions and 111 deletions.
diff --git a/src/ast/variables/LocalVariable.js b/src/ast/variables/LocalVariable.js
@@ -1,8 +1,8 @@
 import Variable from './Variable';
 import VariableShapeTracker from './VariableShapeTracker';
 
-// To avoid infinite recursions
-const MAX_PATH_LENGTH = 6;
+// To avoid exponential performance degradation for complex object manipulations
+const MAX_PATH_LENGTH = 2;
 
 export default class LocalVariable extends Variable {
 	constructor ( name, declarator, init ) {
@@ -54,9 +54,9 @@ export default class LocalVariable extends Variable {
 	hasEffectsWhenAccessedAtPath ( path, options ) {
 		return path.length > MAX_PATH_LENGTH
 			|| this.boundExpressions.someAtPath( path, ( relativePath, node ) =>
-				!options.hasNodeBeenAccessedAtPath( relativePath, node ) && node
-					.hasEffectsWhenAccessedAtPath( relativePath,
-						options.addAccessedNodeAtPath( relativePath, node ) ) );
+				relativePath.length > 0
+				&& !options.hasNodeBeenAccessedAtPath( relativePath, node )
+				&& node.hasEffectsWhenAccessedAtPath( relativePath, options.addAccessedNodeAtPath( relativePath, node ) ) );
 	}
 
 	hasEffectsWhenAssignedAtPath ( path, options ) {

diff --git a/src/ast/variables/VariableShapeTracker.js b/src/ast/variables/VariableShapeTracker.js
@@ -4,20 +4,22 @@ const SET_KEY = { type: 'SET_KEY' };
 export const UNKNOWN_KEY = { type: 'UNKNOWN_KEY' };
 
 const UNKNOWN_ASSIGNMENTS = new Map( [ [ SET_KEY, new Set( [ UNKNOWN_ASSIGNMENT ] ) ] ] );
+const UNKNOWN_KEY_ASSIGNMENT = [ UNKNOWN_KEY, { toString: ( path = '' ) => path + '[[UNKNOWN_KEY]]' } ];
 
 export default class VariableShapeTracker {
 	constructor () {
 		this._assignments = new Map( [ [ SET_KEY, new Set() ] ] );
 	}
 
 	addAtPath ( path, assignment ) {
-		if ( this._assignments === UNKNOWN_ASSIGNMENTS ) return;
-		if ( path.length === 0 ) {
-			if ( assignment === UNKNOWN_ASSIGNMENT ) {
-				this._assignments = UNKNOWN_ASSIGNMENTS;
-			} else {
-				this._assignments.get( SET_KEY ).add( assignment );
-			}
+		if ( this._assignments === UNKNOWN_ASSIGNMENTS
+			|| (path.length > 0 && this._assignments.has( UNKNOWN_KEY ) ) ) return;
+		if ( path.length === 0 && assignment === UNKNOWN_ASSIGNMENT ) {
+			this._assignments = UNKNOWN_ASSIGNMENTS;
+		} else if ( path[ 0 ] === UNKNOWN_KEY ) {
+			this._assignments = new Map( [ [ SET_KEY, this._assignments.get( SET_KEY ) ], UNKNOWN_KEY_ASSIGNMENT ] );
+		} else if ( path.length === 0 ) {
+			this._assignments.get( SET_KEY ).add( assignment );
 		} else {
 			const [ nextPath, ...remainingPath ] = path;
 			if ( !this._assignments.has( nextPath ) ) {
@@ -30,35 +32,26 @@ export default class VariableShapeTracker {
 	forEachAtPath ( path, callback ) {
 		const [ nextPath, ...remainingPath ] = path;
 		this._assignments.get( SET_KEY ).forEach( assignment => callback( path, assignment ) );
-		if ( path.length > 0 ) {
-			if ( nextPath === UNKNOWN_KEY ) {
-				this._assignments.forEach( ( assignment, subPath ) => {
-					if ( subPath !== SET_KEY ) {
-						assignment.forEachAtPath( remainingPath, callback );
-					}
-				} );
-			} else {
-				if ( this._assignments.has( nextPath ) ) {
-					this._assignments.get( nextPath ).forEachAtPath( remainingPath, callback );
-				}
-				if ( this._assignments.has( UNKNOWN_KEY ) ) {
-					this._assignments.get( UNKNOWN_KEY ).forEachAtPath( remainingPath, callback );
-				}
-			}
+		if ( path.length > 0
+			&& nextPath !== UNKNOWN_KEY
+			&& !this._assignments.has( UNKNOWN_KEY )
+			&& this._assignments.has( nextPath ) ) {
+			this._assignments.get( nextPath ).forEachAtPath( remainingPath, callback );
 		}
 	}
 
 	forEachAssignedToPath ( path, callback ) {
+		if ( this._assignments === UNKNOWN_ASSIGNMENTS || this._assignments.has( UNKNOWN_KEY ) ) return;
 		if ( path.length > 0 ) {
 			const [ nextPath, ...remainingPath ] = path;
+			if ( nextPath === UNKNOWN_KEY || this._assignments.has( UNKNOWN_KEY ) ) return;
 			if ( this._assignments.has( nextPath ) ) {
 				this._assignments.get( nextPath ).forEachAssignedToPath( remainingPath, callback );
 			}
 		} else {
+			this._assignments.get( SET_KEY ).forEach( assignment => callback( [], assignment ) );
 			this._assignments.forEach( ( assignment, subPath ) => {
-				if ( subPath === SET_KEY ) {
-					assignment.forEach( subAssignment => callback( [], subAssignment ) );
-				} else {
+				if ( subPath !== SET_KEY ) {
 					assignment.forEachAssignedToPath( [],
 						( relativePath, assignment ) => callback( [ subPath, ...relativePath ], assignment ) );
 				}
@@ -71,6 +64,7 @@ export default class VariableShapeTracker {
 		if ( path.length === 0 ) {
 			return this._assignments.get( SET_KEY ).has( assignment );
 		} else {
+			if ( this._assignments.has( UNKNOWN_KEY ) ) return true;
 			const [ nextPath, ...remainingPath ] = path;
 			if ( !this._assignments.has( nextPath ) ) {
 				return false;
@@ -85,25 +79,16 @@ export default class VariableShapeTracker {
 			|| (
 				path.length > 0
 				&& (
-					(nextPath === UNKNOWN_KEY
-						&& Array.from( this._assignments ).some( ( [ subPath, assignment ] ) => {
-							if ( subPath !== SET_KEY ) {
-								return assignment.someAtPath( remainingPath, predicateFunction );
-							}
-						} ))
-					|| (nextPath !== UNKNOWN_KEY
-						&& (
-							(this._assignments.has( nextPath )
-								&& this._assignments.get( nextPath ).someAtPath( remainingPath, predicateFunction ))
-							|| (this._assignments.has( UNKNOWN_KEY )
-								&& this._assignments.get( UNKNOWN_KEY ).someAtPath( remainingPath, predicateFunction ))
-						))
+					(nextPath === UNKNOWN_KEY || this._assignments.has( UNKNOWN_KEY )
+						? predicateFunction( remainingPath, UNKNOWN_ASSIGNMENT )
+						: this._assignments.has( nextPath )
+						&& this._assignments.get( nextPath ).someAtPath( remainingPath, predicateFunction ))
 				)
 			);
 	}
 
 	// For debugging purposes
-	toString ( pathString = '' ) {
+	toString ( pathString = '/' ) {
 		return Array.from( this._assignments ).map( ( [ subPath, subAssignment ] ) => subPath === SET_KEY
 			? Array.from( subAssignment ).map( assignment => pathString + assignment.toString() ).join( '\n' )
 			: subAssignment.toString( pathString + subPath + ': ' ) ).join( '\n' );

diff --git a/test/form/samples/computed-member-expression-assignments/main.js b/test/form/samples/computed-member-expression-assignments/main.js
@@ -17,7 +17,3 @@ retained5[ 'f' + 'oo' ]();
 
 const removed1 = { foo: {} };
 removed1.foo[ 'b' + 'ar' ] = 1;
-
-const removed2 = { foo: function () {} };
-removed2[ 'f' + 'oo' ] = function () {this.x = 1;};
-const result2 = new removed2.foo();
diff --git a/test/form/samples/side-effects-getters-and-setters/_expected/amd.js b/test/form/samples/side-effects-getters-and-setters/_expected/amd.js
@@ -11,7 +11,6 @@ define(function () { 'use strict';
 	};
 
 	const retained1b = retained1a.effect;
-	const retained1c = retained1a[ 'eff' + 'ect' ];
 
 	const retained3 = {
 		set effect ( value ) {
@@ -21,14 +20,6 @@ define(function () { 'use strict';
 
 	retained3.effect = 'retained';
 
-	const retained4 = {
-		set effect ( value ) {
-			console.log( value );
-		}
-	};
-
-	retained4[ 'eff' + 'ect' ] = 'retained';
-
 	const retained7 = {
 		foo: () => {},
 		get foo () {

diff --git a/test/form/samples/side-effects-getters-and-setters/_expected/cjs.js b/test/form/samples/side-effects-getters-and-setters/_expected/cjs.js
@@ -11,7 +11,6 @@ const retained1a = {
 };
 
 const retained1b = retained1a.effect;
-const retained1c = retained1a[ 'eff' + 'ect' ];
 
 const retained3 = {
 	set effect ( value ) {
@@ -21,14 +20,6 @@ const retained3 = {
 
 retained3.effect = 'retained';
 
-const retained4 = {
-	set effect ( value ) {
-		console.log( value );
-	}
-};
-
-retained4[ 'eff' + 'ect' ] = 'retained';
-
 const retained7 = {
 	foo: () => {},
 	get foo () {

diff --git a/test/form/samples/side-effects-getters-and-setters/_expected/es.js b/test/form/samples/side-effects-getters-and-setters/_expected/es.js
@@ -9,7 +9,6 @@ const retained1a = {
 };
 
 const retained1b = retained1a.effect;
-const retained1c = retained1a[ 'eff' + 'ect' ];
 
 const retained3 = {
 	set effect ( value ) {
@@ -19,14 +18,6 @@ const retained3 = {
 
 retained3.effect = 'retained';
 
-const retained4 = {
-	set effect ( value ) {
-		console.log( value );
-	}
-};
-
-retained4[ 'eff' + 'ect' ] = 'retained';
-
 const retained7 = {
 	foo: () => {},
 	get foo () {

diff --git a/test/form/samples/side-effects-getters-and-setters/_expected/iife.js b/test/form/samples/side-effects-getters-and-setters/_expected/iife.js
@@ -12,7 +12,6 @@
 	};
 
 	const retained1b = retained1a.effect;
-	const retained1c = retained1a[ 'eff' + 'ect' ];
 
 	const retained3 = {
 		set effect ( value ) {
@@ -22,14 +21,6 @@
 
 	retained3.effect = 'retained';
 
-	const retained4 = {
-		set effect ( value ) {
-			console.log( value );
-		}
-	};
-
-	retained4[ 'eff' + 'ect' ] = 'retained';
-
 	const retained7 = {
 		foo: () => {},
 		get foo () {

diff --git a/test/form/samples/side-effects-getters-and-setters/_expected/umd.js b/test/form/samples/side-effects-getters-and-setters/_expected/umd.js
@@ -15,7 +15,6 @@
 	};
 
 	const retained1b = retained1a.effect;
-	const retained1c = retained1a[ 'eff' + 'ect' ];
 
 	const retained3 = {
 		set effect ( value ) {
@@ -25,14 +24,6 @@
 
 	retained3.effect = 'retained';
 
-	const retained4 = {
-		set effect ( value ) {
-			console.log( value );
-		}
-	};
-
-	retained4[ 'eff' + 'ect' ] = 'retained';
-
 	const retained7 = {
 		foo: () => {},
 		get foo () {

diff --git a/test/form/samples/side-effects-getters-and-setters/main.js b/test/form/samples/side-effects-getters-and-setters/main.js
@@ -10,7 +10,6 @@ const retained1a = {
 
 const removed1 = retained1a.noEffect;
 const retained1b = retained1a.effect;
-const retained1c = retained1a[ 'eff' + 'ect' ];
 
 const removed2a = {
 	get shadowedEffect () {
@@ -34,14 +33,6 @@ const retained3 = {
 
 retained3.effect = 'retained';
 
-const retained4 = {
-	set effect ( value ) {
-		console.log( value );
-	}
-};
-
-retained4[ 'eff' + 'ect' ] = 'retained';
-
 const removed5 = {
 	set noEffect ( value ) {
 		const x = value;

diff --git a/test/form/samples/side-effects-object-literal-mutation/main.js b/test/form/samples/side-effects-object-literal-mutation/main.js
@@ -25,10 +25,6 @@ const removed2 = { x: {} };
 removed2.x = 99;
 removed2.x.y = 2;
 
-const removed3 = { x: { y: {} } };
-removed3.x = { y: {} };
-removed3.x.y.z = 3;
-
 const retained6 = { x: { y: {} } };
 retained6.x = {};
 retained6.x.y.z = 3;
@@ -37,8 +33,3 @@ const retained7 = { x: { y: globalVar } };
 const retained8 = { x: { y: {} } };
 retained8.x = retained7.x;
 retained8.x.y.z = 3;
-
-const removed4 = {a: { x: { y: globalVar } }};
-const removed5 = { x: { y: {} } };
-removed5.x = removed4.a.x;
-removed5.x.y = 2;