Framework: Hoist transformed function expression from AsyncLoad compo…

…nents
Automattic · Oct 28, 2016 · c26d2c9 · c26d2c9
1 parent 96f2726
commit c26d2c9
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 9 deletions.
diff --git a/server/bundler/babel/babel-plugin-transform-wpcalypso-async/index.js b/server/bundler/babel/babel-plugin-transform-wpcalypso-async/index.js
@@ -4,6 +4,21 @@
 const kebabCase = require( 'lodash/kebabCase' );
 
 module.exports = ( { types: t } ) => {
+	/**
+	 * Nested visitor for `require` function expression hoisting. This is
+	 * assigned here as a shared reference for optimized path traversal.
+	 *
+	 * @see https://github.com/thejameskyle/babel-handbook/blob/master/translations/en/plugin-handbook.md#optimizing-nested-visitors
+	 * @type {Object}
+	 */
+	const asyncAttributeVisitor = {
+		FunctionExpression( path ) {
+			// Hoist using the parent JSXAttribute's scope, since the scopes
+			// from AST parse stage are not valid for replacement expression
+			path.hoist( this.scope );
+		}
+	};
+
 	return {
 		visitor: {
 			JSXAttribute( path ) {
@@ -29,16 +44,17 @@ module.exports = ( { types: t } ) => {
 				// Replace prop string with function which, when invoked, calls
 				// asyncRequire. The asyncRequire call is transformed by the
 				// CallExpression visitor in this plugin
-				const newValue = t.jSXExpressionContainer(
+				path.replaceWith( t.jSXAttribute( name, t.jSXExpressionContainer(
 					t.functionExpression( null, [ t.identifier( 'callback' ) ], t.blockStatement( [
 						t.expressionStatement( t.callExpression( t.identifier( 'asyncRequire' ), [
 							value,
 							t.identifier( 'callback' )
 						] ) )
 					] ) )
-				);
+				) ) );
 
-				path.replaceWith( t.jSXAttribute( name, newValue ) );
+				// Traverse replacement attribute to hoist function expression
+				path.traverse( asyncAttributeVisitor, { scope: path.scope } );
 			},
 			CallExpression( path, state ) {
 				if ( 'asyncRequire' !== path.node.callee.name ) {

diff --git a/server/bundler/babel/babel-plugin-transform-wpcalypso-async/test/index.js b/server/bundler/babel/babel-plugin-transform-wpcalypso-async/test/index.js
@@ -33,18 +33,18 @@ describe( 'babel-plugin-transform-wpcalypso-async', () => {
 		} );
 
 		context( 'async', () => {
-			it( 'should replace a require string prop', () => {
-				const code = transform( '<AsyncLoad require="foo" />' );
+			it( 'should replace a require string prop with hoisting', () => {
+				const code = transform( 'export default () => <AsyncLoad require="foo" />;' );
 
-				expect( code ).to.equal( '<AsyncLoad require={function (callback) {\n  require.ensure("foo", function (require) {\n    callback(require("foo"));\n  }, "async-load-foo");\n}} />;' );
+				expect( code ).to.equal( 'var _ref = function (callback) {\n  require.ensure("foo", function (require) {\n    callback(require("foo"));\n  }, "async-load-foo");\n};\n\nexport default (() => <AsyncLoad require={_ref} />);' );
 			} );
 		} );
 
 		context( 'sync', () => {
-			it( 'should replace a require string prop', () => {
-				const code = transform( '<AsyncLoad require="foo" />', false );
+			it( 'should replace a require string prop with hoisting', () => {
+				const code = transform( 'export default () => <AsyncLoad require="foo" />;', false );
 
-				expect( code ).to.equal( '<AsyncLoad require={function (callback) {\n  callback(require("foo"));\n}} />;' );
+				expect( code ).to.equal( 'var _ref = function (callback) {\n  callback(require("foo"));\n};\n\nexport default (() => <AsyncLoad require={_ref} />);' );
 			} );
 		} );
 	} );