Clean up code, improve performance and resolve #1850

rollup · Feb 2, 2018 · 504e816 · 504e816
1 parent 5e6058d
commit 504e816
Show file tree

Hide file tree

Showing 16 changed files with 183 additions and 36 deletions.
diff --git a/src/ast/nodes/ExportDefaultDeclaration.ts b/src/ast/nodes/ExportDefaultDeclaration.ts
@@ -6,25 +6,24 @@ import Identifier from './Identifier';
 import MagicString from 'magic-string';
 import { NodeType } from './NodeType';
 import { NodeRenderOptions, RenderOptions } from '../../Module';
+import { findFirstOccurrenceOutsideComment } from '../../utils/renderHelpers';
 
-function buildRegexWithSpaces (re: RegExp) {
-	const spaceOrComment =
-		'(?:' +
-		[
-			/\s/.source, // Space
-			/\/\/.*[\n\r]/.source, // Single line comment
-			/\/\*[^]*?\*\//.source // Multiline comment. There is [^] instead of . because it also matches \n
-		].join('|') +
-		')';
-	return new RegExp(re.source.replace(/\s|\\s/g, spaceOrComment), re.flags);
+// The header ends at the first white-space or comment after "default"
+function getDeclarationStart (code: string) {
+	const headerLength = findFirstOccurrenceOutsideComment(code, 'default') + 7;
+	return headerLength + code.slice(headerLength).search(/\S|\/\*/);
 }
 
-const sourceRE = {
-	exportDefault: buildRegexWithSpaces(/^ *export +default */),
-	declarationHeader: buildRegexWithSpaces(
-		/^ *export +default +(?:(?:async +)?function(?: *\*)?|class)/
-	)
-};
+function getIdInsertPosition (code: string, declarationKeyword: string) {
+	const declarationEnd = findFirstOccurrenceOutsideComment(code, declarationKeyword) + declarationKeyword.length;
+	code = code.slice(declarationEnd);
+	code = code.slice(0, findFirstOccurrenceOutsideComment(code, '{'));
+	const generatorStarPos = findFirstOccurrenceOutsideComment(code, '*');
+	if (generatorStarPos === 0) {
+		return declarationEnd + (code[0] === '*' ? 1 : 0);
+	}
+	return declarationEnd + generatorStarPos + 1;
+}
 
 export default class ExportDefaultDeclaration extends NodeBase {
 	type: NodeType.ExportDefaultDeclaration;
@@ -54,11 +53,12 @@ export default class ExportDefaultDeclaration extends NodeBase {
 	}
 
 	render (code: MagicString, options: RenderOptions, nodeRenderOptions: NodeRenderOptions = {}) {
-		// paren workaround: find first non-whitespace character position after `export default`
-		const declarationStart = this.start + code.original.slice(this.start, this.end).match(sourceRE.exportDefault)[0].length;
+		const declarationStart = this.start + getDeclarationStart(code.original.slice(this.start, this.end));
 
-		if (isFunctionDeclaration(this.declaration) || isClassDeclaration(this.declaration)) {
-			this.renderFunctionOrClassDeclaration(code, declarationStart, this.declaration.id === null, options);
+		if (isFunctionDeclaration(this.declaration)) {
+			this.renderDeclaration(code, declarationStart, 'function', this.declaration.id === null, options);
+		} else if (isClassDeclaration(this.declaration)) {
+			this.renderDeclaration(code, declarationStart, 'class', this.declaration.id === null, options);
 		} else if (this.variable.getOriginalVariableName() === this.variable.getName()) {
 			// Remove altogether to prevent re-declaring the same variable
 			code.remove(nodeRenderOptions.start || this.start, nodeRenderOptions.end || this.end);
@@ -72,13 +72,15 @@ export default class ExportDefaultDeclaration extends NodeBase {
 		super.render(code, options);
 	}
 
-	private renderFunctionOrClassDeclaration (code: MagicString, declarationStart: number, needsId: boolean, options: RenderOptions) {
+	private renderDeclaration (
+		code: MagicString, declarationStart: number, declarationKeyword: string, needsId: boolean, options: RenderOptions
+	) {
 		const name = this.variable.getName();
 		// Remove `export default`
 		code.remove(this.start, declarationStart);
 
 		if (needsId) {
-			const idInsertPos = this.start + code.original.slice(this.start, this.end).match(sourceRE.declarationHeader)[0].length;
+			const idInsertPos = declarationStart + getIdInsertPosition(code.original.slice(declarationStart, this.end), declarationKeyword);
 			code.appendLeft(idInsertPos, ` ${name}`);
 		}
 		if (options.systemBindings && isClassDeclaration(this.declaration)) {

diff --git a/src/utils/renderHelpers.ts b/src/utils/renderHelpers.ts
@@ -3,12 +3,14 @@ import MagicString from 'magic-string';
 import { RenderOptions } from '../Module';
 import { NodeType } from '../ast/nodes/NodeType';
 
-function findFirstLineBreakOutsideComment (code: string) {
+// Note that if the string is not found, "0" is returned instead of e.g. "-1"
+// as this seems to work best for the main use case
+export function findFirstOccurrenceOutsideComment (code: string, searchString: string) {
 	let codeStart = 0;
 	let commentStart, commentLength, lineBreakPos;
 	while (true) {
 		commentStart = code.indexOf('/*');
-		lineBreakPos = (~commentStart ? code.slice(0, commentStart) : code).indexOf('\n');
+		lineBreakPos = (~commentStart ? code.slice(0, commentStart) : code).indexOf(searchString);
 		if (~lineBreakPos || !~commentStart) {
 			break;
 		}
@@ -24,17 +26,15 @@ export function renderStatementBlock (statements: Node[], code: MagicString, sta
 	if (statements.length === 0) return;
 	let currentNode, currentNodeStart;
 	let nextNode = statements[0];
-	// TODO we need to work around a bug in magic-string when nodeStart = start = 0
-	let nextNodeStart = start + (nextNode.start === start ? 0 : findFirstLineBreakOutsideComment(code.slice(start, nextNode.start)));
+	let nextNodeStart = start + findFirstOccurrenceOutsideComment(code.original.slice(start, nextNode.start), '\n');
 
 	for (let nextIndex = 1; nextIndex <= statements.length; nextIndex++) {
 		currentNode = nextNode;
 		currentNodeStart = nextNodeStart;
 		nextNode = statements[nextIndex];
-		nextNodeStart = currentNode.end + (findFirstLineBreakOutsideComment(code.slice(
-			currentNode.end,
-			nextNode !== undefined ? nextNode.start : end
-		)));
+		nextNodeStart = currentNode.end + (findFirstOccurrenceOutsideComment(code.original.slice(
+			currentNode.end, nextNode !== undefined ? nextNode.start : end
+		), '\n'));
 		if (!currentNode.included && currentNode.type !== NodeType.IfStatement) {
 			code.remove(currentNodeStart, nextNodeStart);
 		} else {

diff --git a/test/form/samples/export-default-anonymous-declarations/_expected/amd.js b/test/form/samples/export-default-anonymous-declarations/_expected/amd.js
@@ -1,5 +1,9 @@
 define(function () { 'use strict';
 
+  // jsjjjsjjjjsjs
+
+  /* jsjjddjksj */ // Too many comments lol
+
   function Fn
 
   //iian iaouns
@@ -12,6 +16,11 @@ define(function () { 'use strict';
     console.log("Foo");
   }
 
+  // jsjjjsjjjjsjs
+
+  /* jsjjddjksj
+  */ // Too many comments lol
+
   async /* [no LineTerminator here] */ function Async
 
   //iian iaouns
@@ -21,6 +30,10 @@ define(function () { 'use strict';
     console.log("Foo");
   }
 
+  // jsjjjsjjjjsjs
+
+  /* jsjjddjksj */ // Too many comments lol
+
   function
 
   /* oiasnpiueno */
@@ -38,6 +51,10 @@ define(function () { 'use strict';
     console.log("Foo");
   }
 
+  // jsjjjsjjjjsjs
+
+  /* jsjjddjksj */ // Too many comments lol
+
   class Class
 
   /* oiasnpiueno */
@@ -55,9 +72,19 @@ define(function () { 'use strict';
     }
   }
 
+  /* 2 */function spacelessFn/* 3 */()/* 4 */{
+    console.log("Foo");
+  }
+
+  function* spacelessGenerator(){
+    console.log("Foo");
+  }
+
   Fn();
   Async();
   Generator();
   new Class();
+  spacelessFn();
+  spacelessGenerator();
 
 });
diff --git a/test/form/samples/export-default-anonymous-declarations/_expected/cjs.js b/test/form/samples/export-default-anonymous-declarations/_expected/cjs.js
@@ -1,5 +1,9 @@
 'use strict';
 
+// jsjjjsjjjjsjs
+
+/* jsjjddjksj */ // Too many comments lol
+
 function Fn
 
 //iian iaouns
@@ -12,6 +16,11 @@ function Fn
   console.log("Foo");
 }
 
+// jsjjjsjjjjsjs
+
+/* jsjjddjksj
+*/ // Too many comments lol
+
 async /* [no LineTerminator here] */ function Async
 
 //iian iaouns
@@ -21,6 +30,10 @@ async /* [no LineTerminator here] */ function Async
   console.log("Foo");
 }
 
+// jsjjjsjjjjsjs
+
+/* jsjjddjksj */ // Too many comments lol
+
 function
 
 /* oiasnpiueno */
@@ -38,6 +51,10 @@ function
   console.log("Foo");
 }
 
+// jsjjjsjjjjsjs
+
+/* jsjjddjksj */ // Too many comments lol
+
 class Class
 
 /* oiasnpiueno */
@@ -55,7 +72,17 @@ class Class
   }
 }
 
+/* 2 */function spacelessFn/* 3 */()/* 4 */{
+  console.log("Foo");
+}
+
+function* spacelessGenerator(){
+  console.log("Foo");
+}
+
 Fn();
 Async();
 Generator();
 new Class();
+spacelessFn();
+spacelessGenerator();
diff --git a/test/form/samples/export-default-anonymous-declarations/_expected/es.js b/test/form/samples/export-default-anonymous-declarations/_expected/es.js
@@ -1,3 +1,7 @@
+// jsjjjsjjjjsjs
+
+/* jsjjddjksj */ // Too many comments lol
+
 function Fn
 
 //iian iaouns
@@ -10,6 +14,11 @@ function Fn
   console.log("Foo");
 }
 
+// jsjjjsjjjjsjs
+
+/* jsjjddjksj
+*/ // Too many comments lol
+
 async /* [no LineTerminator here] */ function Async
 
 //iian iaouns
@@ -19,6 +28,10 @@ async /* [no LineTerminator here] */ function Async
   console.log("Foo");
 }
 
+// jsjjjsjjjjsjs
+
+/* jsjjddjksj */ // Too many comments lol
+
 function
 
 /* oiasnpiueno */
@@ -36,6 +49,10 @@ function
   console.log("Foo");
 }
 
+// jsjjjsjjjjsjs
+
+/* jsjjddjksj */ // Too many comments lol
+
 class Class
 
 /* oiasnpiueno */
@@ -53,7 +70,17 @@ class Class
   }
 }
 
+/* 2 */function spacelessFn/* 3 */()/* 4 */{
+  console.log("Foo");
+}
+
+function* spacelessGenerator(){
+  console.log("Foo");
+}
+
 Fn();
 Async();
 Generator();
 new Class();
+spacelessFn();
+spacelessGenerator();
diff --git a/test/form/samples/export-default-anonymous-declarations/_expected/iife.js b/test/form/samples/export-default-anonymous-declarations/_expected/iife.js
@@ -1,6 +1,10 @@
 (function () {
   'use strict';
 
+  // jsjjjsjjjjsjs
+
+  /* jsjjddjksj */ // Too many comments lol
+
   function Fn
 
   //iian iaouns
@@ -13,6 +17,11 @@
     console.log("Foo");
   }
 
+  // jsjjjsjjjjsjs
+
+  /* jsjjddjksj
+  */ // Too many comments lol
+
   async /* [no LineTerminator here] */ function Async
 
   //iian iaouns
@@ -22,6 +31,10 @@
     console.log("Foo");
   }
 
+  // jsjjjsjjjjsjs
+
+  /* jsjjddjksj */ // Too many comments lol
+
   function
 
   /* oiasnpiueno */
@@ -39,6 +52,10 @@
     console.log("Foo");
   }
 
+  // jsjjjsjjjjsjs
+
+  /* jsjjddjksj */ // Too many comments lol
+
   class Class
 
   /* oiasnpiueno */
@@ -56,9 +73,19 @@
     }
   }
 
+  /* 2 */function spacelessFn/* 3 */()/* 4 */{
+    console.log("Foo");
+  }
+
+  function* spacelessGenerator(){
+    console.log("Foo");
+  }
+
   Fn();
   Async();
   Generator();
   new Class();
+  spacelessFn();
+  spacelessGenerator();
 
 }());