Fix "default" reexport issues in non ESM/System formats (#3084)

* Restructure reexport tests and add failing tests * Properly export default as efault * Use correct variable name when reexporting default + names * Replace some side-effectful "some" with proper loops * Use template strings to handle compact mode * Improve coverage * Retrigger CI * Improve coverage and replace some forEach loops
rollup · Aug 28, 2019 · e2fa18d · e2fa18d
1 parent cc5fd63
commit e2fa18d
Show file tree

Hide file tree

Showing 82 changed files with 580 additions and 162 deletions.
diff --git a/src/finalisers/shared/getExportBlock.ts b/src/finalisers/shared/getExportBlock.ts
@@ -11,28 +11,20 @@ export default function getExportBlock(
 ) {
 	const _ = compact ? '' : ' ';
 	const n = compact ? '' : '\n';
-
 	if (!namedExportsMode) {
 		let local;
-		exports.some(expt => {
-			if (expt.exported === 'default') {
-				local = expt.local;
-				return true;
+		if (exports.length > 0) {
+			local = exports[0].local;
+		} else {
+			for (const dep of dependencies) {
+				if (dep.reexports) {
+					const expt = dep.reexports[0];
+					local =
+						dep.namedExportsMode && expt.imported !== '*' && expt.imported !== 'default'
+							? `${dep.name}.${expt.imported}`
+							: dep.name;
+				}
 			}
-			return false;
-		});
-		// search for reexported default otherwise
-		if (!local) {
-			dependencies.some(dep => {
-				if (!dep.reexports) return false;
-				return dep.reexports.some(expt => {
-					if (expt.reexported === 'default') {
-						local = dep.namedExportsMode && expt.imported !== '*' ? `${dep.name}.${expt.imported}` : dep.name;
-						return true;
-					}
-					return false;
-				});
-			});
 		}
 		return `${mechanism}${local};`;
 	}
@@ -44,7 +36,7 @@ export default function getExportBlock(
 		if (reexports && namedExportsMode) {
 			reexports.forEach(specifier => {
 				if (specifier.reexported === '*') {
-					if (!compact && exportBlock) exportBlock += '\n';
+					if (exportBlock) exportBlock += n;
 					if (specifier.needsLiveBinding) {
 						exportBlock +=
 							`Object.keys(${name}).forEach(function${_}(k)${_}{${n}` +
@@ -63,60 +55,61 @@ export default function getExportBlock(
 		}
 	});
 
-	dependencies.forEach(
-		({ name, imports, reexports, isChunk, namedExportsMode: depNamedExportsMode }) => {
-			if (reexports && namedExportsMode) {
-				reexports.forEach(specifier => {
-					if (specifier.imported === 'default' && !isChunk) {
-						const exportsNamesOrNamespace =
-							(imports && imports.some(specifier => specifier.imported !== 'default')) ||
-							(reexports &&
-								reexports.some(
-									specifier => specifier.imported !== 'default' && specifier.imported !== '*'
-								));
-
-						const reexportsDefaultAsDefault =
-							reexports &&
-							reexports.some(
-								specifier => specifier.imported === 'default' && specifier.reexported === 'default'
-							);
-
-						if (exportBlock && !compact) exportBlock += '\n';
-						if (exportsNamesOrNamespace || reexportsDefaultAsDefault)
-							exportBlock += `exports.${specifier.reexported}${_}=${_}${name}${
-								interop !== false ? '__default' : '.default'
-							};`;
-						else exportBlock += `exports.${specifier.reexported}${_}=${_}${name};`;
-					} else if (specifier.imported !== '*') {
-						if (exportBlock && !compact) exportBlock += '\n';
-						const importName =
-							specifier.imported === 'default' && !depNamedExportsMode
-								? name
-								: `${name}.${specifier.imported}`;
-						exportBlock += specifier.needsLiveBinding
-							? `Object.defineProperty(exports,${_}'${specifier.reexported}',${_}{${n}` +
-							  `${t}enumerable:${_}true,${n}` +
-							  `${t}get:${_}function${_}()${_}{${n}` +
-							  `${t}${t}return ${importName};${n}${t}}${n}});`
-							: `exports.${specifier.reexported}${_}=${_}${importName};`;
-					} else if (specifier.reexported !== '*') {
-						if (exportBlock && !compact) exportBlock += '\n';
+	for (const {
+		name,
+		imports,
+		reexports,
+		isChunk,
+		namedExportsMode: depNamedExportsMode,
+		exportsNames
+	} of dependencies) {
+		if (reexports && namedExportsMode) {
+			for (const specifier of reexports) {
+				if (specifier.imported === 'default' && !isChunk) {
+					if (exportBlock) exportBlock += n;
+					if (
+						exportsNames &&
+						(reexports.some(specifier =>
+							specifier.imported === 'default'
+								? specifier.reexported === 'default'
+								: specifier.imported !== '*'
+						) ||
+							(imports && imports.some(specifier => specifier.imported !== 'default')))
+					) {
+						exportBlock += `exports.${specifier.reexported}${_}=${_}${name}${
+							interop !== false ? '__default' : '.default'
+						};`;
+					} else {
 						exportBlock += `exports.${specifier.reexported}${_}=${_}${name};`;
 					}
-				});
+				} else if (specifier.imported !== '*') {
+					if (exportBlock) exportBlock += n;
+					const importName =
+						specifier.imported === 'default' && !depNamedExportsMode
+							? name
+							: `${name}.${specifier.imported}`;
+					exportBlock += specifier.needsLiveBinding
+						? `Object.defineProperty(exports,${_}'${specifier.reexported}',${_}{${n}` +
+						  `${t}enumerable:${_}true,${n}` +
+						  `${t}get:${_}function${_}()${_}{${n}` +
+						  `${t}${t}return ${importName};${n}${t}}${n}});`
+						: `exports.${specifier.reexported}${_}=${_}${importName};`;
+				} else if (specifier.reexported !== '*') {
+					if (exportBlock) exportBlock += n;
+					exportBlock += `exports.${specifier.reexported}${_}=${_}${name};`;
+				}
 			}
 		}
-	);
+	}
 
-	exports.forEach(expt => {
+	for (const expt of exports) {
 		const lhs = `exports.${expt.exported}`;
 		const rhs = expt.local;
-		if (lhs === rhs) {
-			return;
+		if (lhs !== rhs) {
+			if (exportBlock) exportBlock += n;
+			exportBlock += `${lhs}${_}=${_}${rhs};`;
 		}
-		if (exportBlock && !compact) exportBlock += '\n';
-		exportBlock += `${lhs}${_}=${_}${rhs};`;
-	});
+	}
 
 	return exportBlock;
 }
diff --git a/src/finalisers/shared/getInteropBlock.ts b/src/finalisers/shared/getInteropBlock.ts
@@ -6,21 +6,23 @@ export default function getInteropBlock(
 	options: OutputOptions,
 	varOrConst: string
 ) {
+	const _ = options.compact ? '' : ' ';
+
 	return dependencies
 		.map(({ name, exportsNames, exportsDefault, namedExportsMode }) => {
-			if (!namedExportsMode) return;
-
-			if (!exportsDefault || options.interop === false) return null;
+			if (!namedExportsMode || !exportsDefault || options.interop === false) return null;
 
 			if (exportsNames) {
-				if (options.compact)
-					return `${varOrConst} ${name}__default='default'in ${name}?${name}['default']:${name};`;
-				return `${varOrConst} ${name}__default = 'default' in ${name} ? ${name}['default'] : ${name};`;
+				return (
+					`${varOrConst} ${name}__default${_}=${_}'default'${_}in ${name}${_}?` +
+					`${_}${name}['default']${_}:${_}${name};`
+				);
 			}
 
-			if (options.compact)
-				return `${name}=${name}&&${name}.hasOwnProperty('default')?${name}['default']:${name};`;
-			return `${name} = ${name} && ${name}.hasOwnProperty('default') ? ${name}['default'] : ${name};`;
+			return (
+				`${name}${_}=${_}${name}${_}&&${_}${name}.hasOwnProperty('default')${_}?` +
+				`${_}${name}['default']${_}:${_}${name};`
+			);
 		})
 		.filter(Boolean)
 		.join(options.compact ? '' : '\n');

diff --git a/test/form/samples/namespace-import-reexport-2/_config.js b/test/form/samples/namespace-import-reexport-2/_config.js
@@ -0,0 +1,10 @@
+module.exports = {
+	description: 'properly associate or shadow variables in and around functions',
+	options: {
+		external: ['external1', 'external2'],
+		output: {
+			globals: { external1: 'external1', external2: 'external2' },
+			name: 'iife'
+		}
+	}
+};
diff --git a/test/form/samples/namespace-import-reexport-2/_expected/amd.js b/test/form/samples/namespace-import-reexport-2/_expected/amd.js
@@ -0,0 +1,15 @@
+define(['exports', 'external1', 'external2'], function (exports, external1, external2) { 'use strict';
+
+
+
+	Object.defineProperty(exports, 'x', {
+		enumerable: true,
+		get: function () {
+			return external1.x;
+		}
+	});
+	exports.ext = external2;
+
+	Object.defineProperty(exports, '__esModule', { value: true });
+
+});
diff --git a/test/form/samples/namespace-import-reexport-2/_expected/cjs.js b/test/form/samples/namespace-import-reexport-2/_expected/cjs.js
@@ -0,0 +1,16 @@
+'use strict';
+
+Object.defineProperty(exports, '__esModule', { value: true });
+
+var external1 = require('external1');
+var external2 = require('external2');
+
+
+
+Object.defineProperty(exports, 'x', {
+	enumerable: true,
+	get: function () {
+		return external1.x;
+	}
+});
+exports.ext = external2;
diff --git a/test/form/samples/namespace-import-reexport-2/_expected/es.js b/test/form/samples/namespace-import-reexport-2/_expected/es.js
@@ -0,0 +1,3 @@
+export { x } from 'external1';
+import * as external2 from 'external2';
+export { external2 as ext };
diff --git a/test/form/samples/namespace-import-reexport-2/_expected/iife.js b/test/form/samples/namespace-import-reexport-2/_expected/iife.js
@@ -0,0 +1,16 @@
+var iife = (function (exports, external1, external2) {
+	'use strict';
+
+
+
+	Object.defineProperty(exports, 'x', {
+		enumerable: true,
+		get: function () {
+			return external1.x;
+		}
+	});
+	exports.ext = external2;
+
+	return exports;
+
+}({}, external1, external2));
diff --git a/test/form/samples/namespace-import-reexport-2/_expected/system.js b/test/form/samples/namespace-import-reexport-2/_expected/system.js
@@ -0,0 +1,15 @@
+System.register('iife', ['external1', 'external2'], function (exports) {
+	'use strict';
+	return {
+		setters: [function (module) {
+			exports('x', module.x);
+		}, function (module) {
+			exports('ext', module);
+		}],
+		execute: function () {
+
+
+
+		}
+	};
+});
diff --git a/test/form/samples/namespace-import-reexport-2/_expected/umd.js b/test/form/samples/namespace-import-reexport-2/_expected/umd.js
@@ -0,0 +1,17 @@
+(function (global, factory) {
+	typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('external1'), require('external2')) :
+	typeof define === 'function' && define.amd ? define(['exports', 'external1', 'external2'], factory) :
+	(global = global || self, factory(global.iife = {}, global.external1, global.external2));
+}(this, function (exports, external1, external2) { 'use strict';
+
+	Object.defineProperty(exports, 'x', {
+		enumerable: true,
+		get: function () {
+			return external1.x;
+		}
+	});
+	exports.ext = external2;
+
+	Object.defineProperty(exports, '__esModule', { value: true });
+
+}));
diff --git a/test/form/samples/namespace-import-reexport-2/main.js b/test/form/samples/namespace-import-reexport-2/main.js
@@ -0,0 +1,3 @@
+export { x } from 'external1';
+import * as ext from 'external2';
+export { ext };
diff --git a/test/form/samples/re-export-default-external-as-default/_config.js b/test/form/samples/re-export-default-external-as-default/_config.js
diff --git a/test/form/samples/re-export-default-external/_config.js b/test/form/samples/re-export-default-external/_config.js
diff --git a/test/form/samples/reexport-external-default-and-name/_config.js b/test/form/samples/reexport-external-default-and-name/_config.js
@@ -0,0 +1,12 @@
+module.exports = {
+	description:
+		'reexports a an external default as a name and imports another name from that dependency',
+	expectedWarnings: ['MIXED_EXPORTS'],
+	options: {
+		external: ['external'],
+		output: {
+			globals: { external: 'external' },
+			name: 'bundle'
+		}
+	}
+};
diff --git a/test/form/samples/reexport-external-default-and-name/_expected/amd.js b/test/form/samples/reexport-external-default-and-name/_expected/amd.js
@@ -0,0 +1,12 @@
+define(['exports', 'external'], function (exports, external) { 'use strict';
+
+	external = external && external.hasOwnProperty('default') ? external['default'] : external;
+
+	const value = 42;
+
+	exports.default = external;
+	exports.value = value;
+
+	Object.defineProperty(exports, '__esModule', { value: true });
+
+});
diff --git a/test/form/samples/reexport-external-default-and-name/_expected/cjs.js b/test/form/samples/reexport-external-default-and-name/_expected/cjs.js
@@ -0,0 +1,12 @@
+'use strict';
+
+Object.defineProperty(exports, '__esModule', { value: true });
+
+function _interopDefault (ex) { return (ex && (typeof ex === 'object') && 'default' in ex) ? ex['default'] : ex; }
+
+var external = _interopDefault(require('external'));
+
+const value = 42;
+
+exports.default = external;
+exports.value = value;
diff --git a/test/form/samples/reexport-external-default-and-name/_expected/es.js b/test/form/samples/reexport-external-default-and-name/_expected/es.js
@@ -0,0 +1,5 @@
+export { default } from 'external';
+
+const value = 42;
+
+export { value };
diff --git a/test/form/samples/reexport-external-default-and-name/_expected/iife.js b/test/form/samples/reexport-external-default-and-name/_expected/iife.js
@@ -0,0 +1,13 @@
+var bundle = (function (exports, external) {
+	'use strict';
+
+	external = external && external.hasOwnProperty('default') ? external['default'] : external;
+
+	const value = 42;
+
+	exports.default = external;
+	exports.value = value;
+
+	return exports;
+
+}({}, external));
diff --git a/test/form/samples/reexport-external-default-and-name/_expected/system.js b/test/form/samples/reexport-external-default-and-name/_expected/system.js
@@ -0,0 +1,13 @@
+System.register('bundle', ['external'], function (exports) {
+	'use strict';
+	return {
+		setters: [function (module) {
+			exports('default', module.default);
+		}],
+		execute: function () {
+
+			const value = exports('value', 42);
+
+		}
+	};
+});