Merge pull request #99 from stealjs/circular

Fix default imports in ES6 to AMD circular references

lib/es6_amd.js

@@ -1,17 +1,28 @@
-var amd_amd = require('./amd_amd');
-var getCompile = require('./es6_compiler');
 var getAst = require("./get_ast");
+var amdToAmd = require("./amd_amd");
+var getCompile = require("./es6_compiler");
+var patchCircularDependencies = require("./patch_circular_dependencies");
 
-module.exports = function(load, options){
+module.exports = function(load, options) {
 	var compile = getCompile(options);
 
-	var result = compile(load.source.toString(), {
-		filename: options.sourceMapFileName || load.address,
-		modules: 'amd',
-		sourceMaps: true
-	}, options);
+	var result = compile(
+		load.source.toString(),
+		{
+			filename: options.sourceMapFileName || load.address,
+			modules: "amd",
+			sourceMaps: true
+		},
+		options
+	);
+
 	load.source = result.code;
 	load.map = result.map;
 	load.ast = getAst(load, options.sourceMapFileName);
-	return amd_amd(load, options);
+
+	if (load.circular && options.patchCircularDependencies) {
+		load.ast = patchCircularDependencies(load.ast);
+	}
+
+	return amdToAmd(load, options);
 };

lib/patch_circular_dependencies.js

@@ -0,0 +1,114 @@
+var types = require("ast-types");
+var first = require("lodash/first");
+var last = require("lodash/last");
+var concat = require("lodash/concat");
+var estemplate = require("estemplate");
+
+var n = types.namedTypes;
+
+module.exports = function(ast) {
+	var variableNames = collectVariableNames(ast);
+
+	appendToDefineFactory(
+		ast,
+		concat(variableNames.map(patchHelperCallTemplate), patchHelperTemplate())
+	);
+
+	return ast;
+};
+
+/**
+ * Collects the identifiers of Babel generated import assignments
+ *
+ * E.g, given the AST of the code below:
+ *
+ *	var _bar2 = _interopRequireDefault('bar');
+ *	var _foo2 = _interopRequireDefault('foo');
+ *
+ * this function will return an array with the AST nodes of the _bar2 and
+ * _foo2 identifiers.
+ *
+ * @param {Object} ast - The code's AST
+ * @return {Array.<Object>} A list of identifiers (AST nodes)
+ */
+function collectVariableNames(ast) {
+	var names = [];
+
+	types.visit(ast, {
+		visitVariableDeclarator: function(path) {
+			var node = path.node;
+
+			if (this.isBabelRequireInteropCall(node.init)) {
+				names.push(node.id);
+			}
+
+			this.traverse(path);
+		},
+
+		isBabelRequireInteropCall: function(node) {
+			return (
+				n.CallExpression.check(node) &&
+				node.callee.name === "_interopRequireDefault"
+			);
+		}
+	});
+
+	return names;
+}
+
+/**
+ * Appends the given node to the `define` factory function body
+ *
+ * MUTATES THE AST
+ *
+ * @param {Object} ast - The AMD module AST
+ * @param {Array.<Object>} nodes - The AST nodes to be appended
+ */
+function appendToDefineFactory(ast, nodes) {
+	types.visit(ast, {
+		visitCallExpression: function(path) {
+			var node = path.node;
+
+			if (this.isDefineIdentifier(node.callee)) {
+				var factory = last(node.arguments);
+				factory.body.body = concat(factory.body.body, nodes);
+				this.abort();
+			}
+
+			this.traverse(path);
+		},
+
+		isDefineIdentifier: function(node) {
+			return n.Identifier.check(node) && node.name === "define";
+		}
+	});
+}
+
+function patchHelperCallTemplate(identifier) {
+	return first(
+		estemplate.compile("_patchCircularDependency(%= name %)")({
+			name: identifier
+		}).body
+	);
+}
+
+function patchHelperTemplate() {
+	return first(
+		estemplate.compile(`
+		function _patchCircularDependency(obj) {
+		  var defaultExport;
+		  Object.defineProperty(obj.default, "default", {
+			set: function(value) {
+			  if (obj.default.__esModule) {
+				obj.default = value;
+			  }
+			  defaultExport = value;
+			},
+			get: function() {
+			  return defaultExport;
+			}
+		  });
+		}
+	`)({}).body
+	);
+}

main.js

@@ -1,7 +1,9 @@
 var bfs = require("./lib/bfs");
-var detect = require("js-module-formats").detect;
 var generate = require("./lib/generate");
 var getAst = require("./lib/get_ast");
+var partial = require("lodash/partial");
+var cloneDeep = require("lodash/cloneDeep");
+var detect = require("js-module-formats").detect;
 var sourceMapFileName = require("./lib/source_map_filename");
 var makeFormatsGraph = require("./lib/make_transforms_formats_graph.js");
 
@@ -32,17 +34,22 @@ function moduleType(source) {
 }
 
 /**
- * Given the path of format transformations, checks if CJS -> AMD is the last step
- * @param {Array.<String>} path - Path of format transformations
- * @return {Boolean} true if CJS -> AMD is the last step
+ * Whether the last step of the transform is from "source" to "dest"
+ * @param {string} source - The source code format
+ * @param {string} dest - The destination format
+ * @param {Array.<string>} path - Path of format transformations
+ * @return {Boolean} true if "source" -> "dest"  is the last step
  */
-function transformsCjsToAmd(path) {
-	var last = path[path.length - 1];
-	var prev = path[path.length - 2];
-
-	return prev === "cjs" && last === "amd";
+function endsWith(source, dest, path) {
+	return (
+		path[path.length - 2] === source &&
+		path[path.length - 1] === dest
+	);
 }
 
+var transformsCjsToAmd = partial(endsWith, "cjs", "amd");
+var transformsEs6ToAmd = partial(endsWith, "es6", "amd");
+
 // transpile.to
 var transpile = {
 	transpilers: transpilers,
@@ -51,9 +58,10 @@ var transpile = {
 		var path = this.able(sourceFormat, destFormat);
 
 		if (!path) {
-			throw new Error(
-				`transpile - unable to transpile ${sourceFormat} to ${destFormat}`
-			);
+			throw new Error([
+				`Unable to transpile '${sourceFormat}' to '${destFormat}'`,
+				`'transpile' does not support '${sourceFormat}' to '${destFormat}' transformations`
+			].join("\n"));
 		}
 
 		// the source format and the dest format are the same, e.g: cjs_cjs
@@ -64,18 +72,13 @@ var transpile = {
 
 		path.push(destFormat);
 
-		var copy = Object.assign({}, load);
+		var copy = cloneDeep(load);
 		var normalize = options.normalize;
 
 		var transpileOptions = options || {};
-		transpileOptions.sourceMapFileName = sourceMapFileName(copy, options);
-
-		// will add the module dependencies using AMD's define dependencies array
-		// this ensure dependencies are loaded if the loaded misses the `require`
-		// calls in the AMD factory body.
-		if (transformsCjsToAmd(path)) {
-			transpileOptions.duplicateCjsDependencies = true;
-		}
+        transpileOptions.sourceMapFileName = sourceMapFileName(copy, options);
+		transpileOptions.duplicateCjsDependencies = transformsCjsToAmd(path);
+		transpileOptions.patchCircularDependencies = transformsEs6ToAmd(path);
 
 		// Create the initial AST
 		if (sourceFormat !== "es6") {

package.json

@@ -12,8 +12,8 @@
     "babel-standalone": "^6.23.1",
     "comparify": "0.2.0",
     "escodegen": "^1.7.0",
-    "estemplate": "^0.5.1",
     "esprima": "^4.0.0",
+    "estemplate": "^0.5.1",
     "estraverse": "4.2.0",
     "js-module-formats": "~0.1.2",
     "js-string-escape": "1.0.1",

test/do-transpile.js

@@ -3,6 +3,7 @@ var fs = require("fs");
 var path = require("path");
 var assert = require("assert");
 var transpile = require("../main");
+var assign = require("lodash/assign");
 
 var readFile = Q.denodeify(fs.readFile);
 var isWindows = /^win/.test(process.platform);
@@ -20,19 +21,24 @@ module.exports = function doTranspile(args) {
 
 	return readFile(srcAddress)
 		.then(function(data) {
-			return transpile.to({
-				name: sourceFileName,
-				address: srcAddress,
-				source: data.toString(),
-				metadata: { format: moduleFormat }
-			}, resultModuleFormat, options);
+			return transpile.to(
+				assign({}, args.load, {
+					name: sourceFileName,
+					address: srcAddress,
+					source: data.toString(),
+					metadata: { format: moduleFormat }
+				}),
+				resultModuleFormat,
+				options
+			);
 		})
 		.then(function(res) {
 			actualCode = res.code;
 			actualMap = res.map && res.map.toString();
 
-			return readFile(path.join(__dirname, "tests", "expected",
-				expectedFileName + ".js"));
+			return readFile(
+				path.join(__dirname, "tests", "expected", expectedFileName + ".js")
+			);
 		})
 		.then(function(data) {
 			var expected = data.toString();
@@ -49,14 +55,23 @@ module.exports = function doTranspile(args) {
 			assert.equal(actualCode, expected, "expected equals result");
 
 			if (options.sourceMaps) {
-				return readFile(path.join(__dirname, "tests", "expected",
-					expectedFileName + ".js.map"));
+				return readFile(
+					path.join(
+						__dirname,
+						"tests",
+						"expected",
+						expectedFileName + ".js.map"
+					)
+				);
 			}
 		})
 		.then(function(expectedMap) {
 			if (expectedMap) {
-				assert.equal(actualMap, expectedMap.toString(),
-					"expected map equals result");
+				assert.equal(
+					actualMap,
+					expectedMap.toString(),
+					"expected map equals result"
+				);
 			}
 		});
 };

test/test.js

@@ -408,6 +408,17 @@ describe("es6 - amd", function() {
 			}
 		});
 	});
+
+	it("works with babel and circular dependencies", function() {
+		return doTranspile({
+			moduleFormat: "es6",
+			sourceFileName: "es6",
+			load: { circular: true },
+			resultModuleFormat: "amd",
+			options: { transpiler: "babel" },
+			expectedFileName: "es6_amd_babel_circular"
+		});
+	});
 });
 
 describe("normalize options", function() {

test/tests/expected/es6_amd_babel_circular.js

@@ -0,0 +1,32 @@
+define([
+    'exports',
+    'basics/amdmodule'
+], function (exports, _amdmodule) {
+    'use strict';
+    Object.defineProperty(exports, '__esModule', { value: true });
+    exports.__useDefault = undefined;
+    var _amdmodule2 = _interopRequireDefault(_amdmodule);
+    function _interopRequireDefault(obj) {
+        return obj && obj.__esModule ? obj : { default: obj };
+    }
+    exports.default = {
+        amdModule: _amdmodule2.default,
+        name: 'es6Module'
+    };
+    var __useDefault = exports.__useDefault = true;
+    _patchCircularDependency(_amdmodule2);
+    function _patchCircularDependency(obj) {
+        var defaultExport;
+        Object.defineProperty(obj.default, 'default', {
+            set: function (value) {
+                if (obj.default.__esModule) {
+                    obj.default = value;
+                }
+                defaultExport = value;
+            },
+            get: function () {
+                return defaultExport;
+            }
+        });
+    }
+});