Ver código fonte

Bender.amd.define() should use the exports object to reduce circ deps problems.

Piotrek Koszuliński 10 anos atrás
pai
commit
13bcff2e4a
2 arquivos alterados com 56 adições e 14 exclusões
  1. 28 10
      dev/bender/plugins/ckeditor5/static/amd.js
  2. 28 4
      tests/bender/amd.js

+ 28 - 10
dev/bender/plugins/ckeditor5/static/amd.js

@@ -76,14 +76,30 @@
 		 * If you need to define a module which has access to other exports or can export more values,
 		 * use the global `define()` function:
 		 *
-		 *		define( bender.amd.getModulePath( name ), [ 'foo', ... ].map( bender.amd.getModulePath ), ( FooModule, ... ) {
+		 *		define( bender.amd.getModulePath( name ), [ 'exports', 'foo', ... ].map( bender.amd.getModulePath ), ( FooModule, ... ) {
 		 *			const FooClass = FooModule.default;
 		 *			const FooOtherProp = FooModule.otherProp;
 		 *
-		 *			return {
-		 *				default: MyClass,
-		 *				otherProp: 1
-		 *			};
+		 *			exports.default = MyClass;
+		 *			exports.otherProp = 1;
+		 *		} );
+		 *
+		 * **Note:** Since this method automatically unwraps modules from the ES6 module object when passing them
+		 * to the `body` function, circular dependencies will not work. If you need them, either use the raw `define()`
+		 * as shown above, or keep all the definitions outside modules and only access the variables from the scope:
+		 *
+		 *		PluginE = createPlugin( 'E' );
+		 *		PluginF = createPlugin( 'F' );
+		 *
+		 *		PluginF.requires = [ PluginE ];
+		 *		PluginE.requires = [ PluginF ];
+		 *
+		 *		bender.amd.define( 'E', [ 'core/plugin', 'F' ], () => {
+		 *			return PluginE;
+		 *		} );
+		 *
+		 *		bender.amd.define( 'F', [ 'core/plugin', 'E' ], () => {
+		 *			return PluginF;
 		 *		} );
 		 *
 		 * @param {String} name Name of the module.
@@ -98,12 +114,14 @@
 
 			const depsPaths = deps.map( bender.amd.getModulePath );
 
-			define( bender.amd.getModulePath( name ), depsPaths, function() {
-				const loadedDeps = Array.from( arguments ).map( ( module ) => module.default );
+			// Use the exports object instead of returning from modules in order to handle circular deps.
+			// http://requirejs.org/docs/api.html#circular
+			depsPaths.unshift( 'exports' );
+
+			define( bender.amd.getModulePath( name ), depsPaths, function( exports ) {
+				const loadedDeps = Array.from( arguments ).slice( 1 ).map( ( module ) => module.default );
 
-				return {
-					default: body.apply( this, loadedDeps )
-				};
+				exports.default = body.apply( this, loadedDeps );
 			} );
 		},
 

+ 28 - 4
tests/bender/amd.js

@@ -3,6 +3,8 @@
  * For licensing, see LICENSE.md.
  */
 
+/* global require */
+
 'use strict';
 
 bender.tools.createSinonSandbox();
@@ -49,12 +51,13 @@ describe( 'bender.amd', () => {
 	describe( 'define()', () => {
 		it( 'defines a module by using global define()', () => {
 			const spy = bender.sinon.spy( window, 'define' );
+			const expectedDeps = [ 'exports' ].concat( [ 'bar', 'ckeditor' ].map( getModulePath ) );
 
 			bender.amd.define( 'test1', [ 'bar', 'ckeditor' ], () => {} );
 
 			expect( spy.calledOnce ).to.be.true;
 			expect( spy.args[ 0 ][ 0 ] ).to.equal( getModulePath( 'test1' ) );
-			expect( spy.args[ 0 ][ 1 ] ).to.deep.equal( [ 'bar', 'ckeditor' ].map( getModulePath ) );
+			expect( spy.args[ 0 ][ 1 ] ).to.deep.equal( expectedDeps );
 		} );
 
 		it( 'maps body args and returned value', () => {
@@ -64,12 +67,13 @@ describe( 'bender.amd', () => {
 			bender.amd.define( 'test2', [ 'bar', 'ckeditor' ], bodySpy );
 
 			const realBody = spy.args[ 0 ][ 2 ];
+			const exportsObj = {};
 
 			expect( realBody ).to.be.a( 'function' );
 
-			const ret = realBody( { default: 'arg' } );
+			realBody( exportsObj, { default: 'arg' } );
 
-			expect( ret ).to.have.property( 'default', 'ret', 'it wraps the ret value with an ES6 module obj' );
+			expect( exportsObj ).to.have.property( 'default', 'ret', 'it wraps the ret value with an ES6 module obj' );
 			expect( bodySpy.calledOnce ).to.be.true;
 			expect( bodySpy.args[ 0 ][ 0 ] ).to.equal( 'arg', 'it unwraps the args' );
 		} );
@@ -81,9 +85,29 @@ describe( 'bender.amd', () => {
 
 			expect( spy.calledOnce ).to.be.true;
 			expect( spy.args[ 0 ][ 0 ] ).to.equal( getModulePath( 'test1' ) );
-			expect( spy.args[ 0 ][ 1 ] ).to.be.empty;
+			expect( spy.args[ 0 ][ 1 ] ).to.deep.equal( [ 'exports' ] );
 			expect( spy.args[ 0 ][ 2 ] ).to.be.a( 'function' );
 		} );
+
+		// Note: this test only checks whether Require.JS doesn't totally fail when creating a circular dependency.
+		// The value of dependencies are not available anyway inside the bender.amd.define() callbacks because
+		// we lose the late-binding by immediately mapping modules to their default exports.
+		it( 'works with circular dependencies', ( done ) => {
+			bender.amd.define( 'test-circular-a', [ 'test-circular-b' ], () => {
+				return 'a';
+			} );
+
+			bender.amd.define( 'test-circular-b', [ 'test-circular-a' ], () => {
+				return 'b';
+			} );
+
+			require( [ 'test-circular-a', 'test-circular-b' ].map( bender.amd.getModulePath ), ( a, b ) => {
+				expect( a ).to.have.property( 'default', 'a' );
+				expect( b ).to.have.property( 'default', 'b' );
+
+				done();
+			} );
+		} );
 	} );
 
 	describe( 'require', () => {