Merge pull request #1787 from nicolo-ribaudo/hoist-anonymous

Hoist default exported anonymous function declarations
rollup · Dec 15, 2017 · 4d0a903 · 4d0a903
2 parents 4324d20 + 40886c3
commit 4d0a903
Show file tree

Hide file tree

Showing 20 changed files with 71 additions and 68 deletions.
diff --git a/src/ast/nodes/ExportDefaultDeclaration.js b/src/ast/nodes/ExportDefaultDeclaration.js
@@ -28,60 +28,48 @@ export default class ExportDefaultDeclaration extends Node {
 		this.variable = this.scope.addExportDefaultDeclaration( this._declarationName || this.module.basename(), this );
 	}
 
-	// TODO this is total chaos, tidy it up
 	render ( code, es ) {
-		const treeshake = this.module.bundle.treeshake;
+		const remove = () => { code.remove( this.leadingCommentStart || this.start, this.next || this.end ); };
+		const removeExportDefault = () => { code.remove( this.start, declaration_start ); };
+
+		const treeshakeable = this.module.bundle.treeshake && !this.included && !this.declaration.included;
 		const name = this.variable.getName( es );
+		const statementStr = code.original.slice( this.start, this.end );
 
 		// paren workaround: find first non-whitespace character position after `export default`
-		let declaration_start;
-		if ( this.declaration ) {
-			const statementStr = code.original.slice( this.start, this.end );
-			declaration_start = this.start + statementStr.match( /^\s*export\s+default\s*/ )[ 0 ].length;
-		}
-
-		if ( this.included || this.declaration.included ) {
-			if ( this.included ) {
-				if ( functionOrClassDeclaration.test( this.declaration.type ) ) {
-					if ( this.declaration.id ) {
-						code.remove( this.start, declaration_start );
-					} else {
-						code.overwrite( this.start, declaration_start, `var ${this.variable.name} = ` );
-						if ( code.original[ this.end - 1 ] !== ';' ) code.appendLeft( this.end, ';' );
-					}
-				}
+		const declaration_start = this.start + statementStr.match( /^\s*export\s+default\s*/ )[ 0 ].length;
 
-				else {
-					if ( this.variable.getOriginalVariableName( es ) === name ) {
-						// prevent `var foo = foo`
-						code.remove( this.leadingCommentStart || this.start, this.next || this.end );
-						return; // don't render children. TODO this seems like a bit of a hack
-					} else {
-						code.overwrite( this.start, declaration_start, `${this.module.bundle.varOrConst} ${name} = ` );
-					}
+		if ( functionOrClassDeclaration.test(this.declaration.type) ) {
+			if ( treeshakeable ) {
+				return remove();
+			}
 
-					this.insertSemicolon( code );
-				}
-			} else {
-				// remove `var foo` from `var foo = bar()`, if `foo` is unused
-				code.remove( this.start, declaration_start );
+			// Add the id to anonymous declarations
+			if ( !this.declaration.id ) {
+				const id_insertPos = this.start + statementStr.match( /^\s*export\s+default\s*(?:function|class)/ )[ 0 ].length;
+				code.appendLeft( id_insertPos, ` ${name}` );
 			}
 
-			super.render( code, es );
+			removeExportDefault();
 		} else {
-			if ( treeshake ) {
-				if ( functionOrClassDeclaration.test( this.declaration.type ) ) {
-					code.remove( this.leadingCommentStart || this.start, this.next || this.end );
-				} else {
-					const hasEffects = this.declaration.hasEffects( ExecutionPathOptions.create() );
-					code.remove( this.start, hasEffects ? declaration_start : this.next || this.end );
-				}
-			} else if ( name === this.declaration.name ) {
-				code.remove( this.start, this.next || this.end );
-			} else {
+			if ( treeshakeable ) {
+				const hasEffects = this.declaration.hasEffects( ExecutionPathOptions.create() );
+				return hasEffects ? removeExportDefault() : remove();
+			}
+
+			// Prevent `var foo = foo`
+			if ( this.variable.getOriginalVariableName( es ) === name ) {
+				return remove();
+			}
+
+			// Only output `var foo =` if `foo` is used
+			if ( this.included ) {
 				code.overwrite( this.start, declaration_start, `${this.module.bundle.varOrConst} ${name} = ` );
+			} else {
+				removeExportDefault();
 			}
-			// code.remove( this.start, this.next || this.end );
 		}
+
+		super.render( code, es );
 	}
 }
diff --git a/test/form/samples/removes-existing-sourcemap-comments/_expected/amd.js b/test/form/samples/removes-existing-sourcemap-comments/_expected/amd.js
@@ -1,8 +1,8 @@
 define(function () { 'use strict';
 
-	var foo = function () {
+	function foo () {
 		return 42;
-	};
+	}
 
 	var str = `
 //# sourceMappingURL=main.js.map

diff --git a/test/form/samples/removes-existing-sourcemap-comments/_expected/cjs.js b/test/form/samples/removes-existing-sourcemap-comments/_expected/cjs.js
@@ -1,8 +1,8 @@
 'use strict';
 
-var foo = function () {
+function foo () {
 	return 42;
-};
+}
 
 var str = `
 //# sourceMappingURL=main.js.map

diff --git a/test/form/samples/removes-existing-sourcemap-comments/_expected/es.js b/test/form/samples/removes-existing-sourcemap-comments/_expected/es.js
@@ -1,6 +1,6 @@
-var foo = function () {
+function foo () {
 	return 42;
-};
+}
 
 var str = `
 //# sourceMappingURL=main.js.map

diff --git a/test/form/samples/removes-existing-sourcemap-comments/_expected/iife.js b/test/form/samples/removes-existing-sourcemap-comments/_expected/iife.js
@@ -1,9 +1,9 @@
 (function () {
 	'use strict';
 
-	var foo = function () {
+	function foo () {
 		return 42;
-	};
+	}
 
 	var str = `
 //# sourceMappingURL=main.js.map

diff --git a/test/form/samples/removes-existing-sourcemap-comments/_expected/umd.js b/test/form/samples/removes-existing-sourcemap-comments/_expected/umd.js
@@ -4,9 +4,9 @@
 	(factory());
 }(this, (function () { 'use strict';
 
-	var foo = function () {
+	function foo () {
 		return 42;
-	};
+	}
 
 	var str = `
 //# sourceMappingURL=main.js.map

diff --git a/test/form/samples/this-in-imports/_expected/amd.js b/test/form/samples/this-in-imports/_expected/amd.js
@@ -8,9 +8,9 @@ define(function () { 'use strict';
 		this.x = 1;
 	}
 
-	var B3 = function () {
+	function B3 () {
 		this.x = 1;
-	};
+	}
 
 	const b1 = B();
 

diff --git a/test/form/samples/this-in-imports/_expected/cjs.js b/test/form/samples/this-in-imports/_expected/cjs.js
@@ -8,9 +8,9 @@ function B$1 () {
 	this.x = 1;
 }
 
-var B3 = function () {
+function B3 () {
 	this.x = 1;
-};
+}
 
 const b1 = B();
 

diff --git a/test/form/samples/this-in-imports/_expected/es.js b/test/form/samples/this-in-imports/_expected/es.js
@@ -6,9 +6,9 @@ function B$1 () {
 	this.x = 1;
 }
 
-var B3 = function () {
+function B3 () {
 	this.x = 1;
-};
+}
 
 const b1 = B();
 

diff --git a/test/form/samples/this-in-imports/_expected/iife.js b/test/form/samples/this-in-imports/_expected/iife.js
@@ -9,9 +9,9 @@
 		this.x = 1;
 	}
 
-	var B3 = function () {
+	function B3 () {
 		this.x = 1;
-	};
+	}
 
 	const b1 = B();
 

diff --git a/test/form/samples/this-in-imports/_expected/umd.js b/test/form/samples/this-in-imports/_expected/umd.js
@@ -12,9 +12,9 @@
 		this.x = 1;
 	}
 
-	var B3 = function () {
+	function B3 () {
 		this.x = 1;
-	};
+	}
 
 	const b1 = B();
 

diff --git a/test/form/samples/unused-called-import/_expected/amd.js b/test/form/samples/unused-called-import/_expected/amd.js
@@ -1,6 +1,6 @@
 define(function () { 'use strict';
 
-	var foo = function() { return 'foo'; };
+	function foo() { return 'foo'; }
 
 	assert.equal( foo(), 'foo' );
 

diff --git a/test/form/samples/unused-called-import/_expected/cjs.js b/test/form/samples/unused-called-import/_expected/cjs.js
@@ -1,5 +1,5 @@
 'use strict';
 
-var foo = function() { return 'foo'; };
+function foo() { return 'foo'; }
 
 assert.equal( foo(), 'foo' );
diff --git a/test/form/samples/unused-called-import/_expected/es.js b/test/form/samples/unused-called-import/_expected/es.js
@@ -1,3 +1,3 @@
-var foo = function() { return 'foo'; };
+function foo() { return 'foo'; }
 
 assert.equal( foo(), 'foo' );
diff --git a/test/form/samples/unused-called-import/_expected/iife.js b/test/form/samples/unused-called-import/_expected/iife.js
@@ -1,7 +1,7 @@
 (function () {
 	'use strict';
 
-	var foo = function() { return 'foo'; };
+	function foo() { return 'foo'; }
 
 	assert.equal( foo(), 'foo' );
 

diff --git a/test/form/samples/unused-called-import/_expected/umd.js b/test/form/samples/unused-called-import/_expected/umd.js
@@ -4,7 +4,7 @@
 	(factory());
 }(this, (function () { 'use strict';
 
-	var foo = function() { return 'foo'; };
+	function foo() { return 'foo'; }
 
 	assert.equal( foo(), 'foo' );
 

diff --git a/test/function/samples/cycles-default-anonymous-function-hoisted/_config.js b/test/function/samples/cycles-default-anonymous-function-hoisted/_config.js
@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'Anonymous function declarations are hoisted'
+};
diff --git a/test/function/samples/cycles-default-anonymous-function-hoisted/f.js b/test/function/samples/cycles-default-anonymous-function-hoisted/f.js
@@ -0,0 +1,4 @@
+import g from "./g.js";
+export default function (fn) {
+  return function (str) { return str || g(fn.name); }
+}
diff --git a/test/function/samples/cycles-default-anonymous-function-hoisted/g.js b/test/function/samples/cycles-default-anonymous-function-hoisted/g.js
@@ -0,0 +1,4 @@
+import f from "./f.js";
+export default f(function fn(str) {
+  return str + str;
+});
diff --git a/test/function/samples/cycles-default-anonymous-function-hoisted/main.js b/test/function/samples/cycles-default-anonymous-function-hoisted/main.js
@@ -0,0 +1,4 @@
+import f from "./f.js";
+import g from "./g.js";
+
+assert.equal(g(), "fn");