resourceloader: Introduce new module state "executing"
authorTimo Tijhof <krinklemail@gmail.com>
Mon, 7 Sep 2015 19:56:06 +0000 (20:56 +0100)
committerTimo Tijhof <krinklemail@gmail.com>
Mon, 7 Sep 2015 20:04:56 +0000 (21:04 +0100)
No longer re-use "loading" to avoid confusion.

Code paths to 'execute' (such as in 'handlePending') already prevent
double execution by only allowing execution to be started when a module
is in state "loaded". So when execute takes ownership by progressing
the state to "executing" (or "loading"), no other code paths will enter
execute until execute itself sets the next state (ready or failed).

Change-Id: I35c8d2bc2da7135456c39d4b513661d2d11fb0f4

resources/src/mediawiki/mediawiki.js
tests/qunit/suites/resources/mediawiki/mediawiki.test.js

index f0af12b..49c350a 100644 (file)
                         *    The module is known to the system but not yet requested.
                         *    Meta data is registered via mw.loader#register. Calls to that method are
                         *    generated server-side by the startup module.
-                        * - `loading` (1):
+                        * - `loading`:
                         *    The module is requested through mw.loader (either directly or as dependency of
                         *    another module). The client will be fetching module contents from the server.
                         *    The contents are then stashed in the registry via mw.loader#implement.
                         *    The module has been requested from the server and stashed via mw.loader#implement.
                         *    If the module has no more dependencies in-fight, the module will be executed
                         *    right away. Otherwise execution is deferred, controlled via #handlePending.
-                        * - `loading` (2):
+                        * - `executing`:
                         *    The module is being executed.
-                        *    TODO: Create a new state instead of re-using the "loading" state.
                         * - `ready`:
                         *    The module has been successfully executed.
                         * - `error`:
                                if ( !hasOwn.call( registry, module ) ) {
                                        throw new Error( 'Module has not been registered yet: ' + module );
                                }
-                               if ( registry[ module ].state === 'registered' ) {
-                                       throw new Error( 'Module has not been requested from the server yet: ' + module );
-                               }
-                               if ( registry[ module ].state === 'loading' ) {
-                                       throw new Error( 'Module has not completed loading yet: ' + module );
-                               }
-                               if ( registry[ module ].state === 'ready' ) {
-                                       throw new Error( 'Module has already been executed: ' + module );
+                               if ( registry[ module ].state !== 'loaded' ) {
+                                       throw new Error( 'Module in state "' + registry[ module ].state + '" may not be executed: ' + module );
                                }
 
-                               // This used to be inside runScript, but since that is now fired asychronously
-                               // (after CSS is loaded) we need to set it here right away. It is crucial that
-                               // when execute() is called this is set synchronously, otherwise modules will get
-                               // executed multiple times as the registry will state that it isn't loading yet.
-                               registry[ module ].state = 'loading';
+                               registry[ module ].state = 'executing';
 
                                runScript = function () {
                                        var script, markModuleReady, nestedAddScript, legacyWait,
index 998887d..86c2eb8 100644 (file)
                                assert.strictEqual( isJsExecuted, undefined, 'script not executed multiple times' );
                                isJsExecuted = true;
 
-                               assert.equal( mw.loader.getState( 'test.implement.import' ), 'loading', 'module state during implement() script execution' );
+                               assert.equal( mw.loader.getState( 'test.implement.import' ), 'executing', 'module state during implement() script execution' );
 
                                $element = $( '<div class="mw-test-implement-import">Foo bar</div>' ).appendTo( '#qunit-fixture' );