Opened 5 years ago

Closed 3 years ago

#18123 closed defect (fixed)

require.undef doesn't work with plugin modules, or modules loaded by plugin modules

Reported by: chuckd Owned by: dylan
Priority: high Milestone: 1.11
Component: Loader Version: 1.9.0
Keywords: Cc:
Blocked By: Blocking:

Description

When unit testing loader plugins, it is often desirable to be able to undef the plugin module and/or the modules loaded by the plugin modules so that they can be reloaded by the unit tests and tested with different initial conditions. The current loader does not support this. The problem is illustrated with the following test case:

foobarPlugin.js:

define(["dojo/has"], function(has) {
	/* Loads a string based on the value of defined features */
	var data = has("foo") ? "foo" : has("bar") ? "bar" : "undefined";
	return {
		load: function(id, parentRequire, loaded) {
			loaded(data);
		}
	}
});

foobarPluginTest.js:

define(["require", "dojo/has"], function(require, has) {
	describe("foobarPluginTest", function() {
		afterEach(function() {
			require.undef("./foobarPlugin!");
			require.undef("./foobarPlugin");
		});
		
		function testIt(expected, fooValue, barValue) {
			has.add("foo", fooValue, true, true);
			has.add("bar", barValue, true, true);
			var pluginData;
			runs(function() {
				require(["./foobarPlugin!"], function(data) {
					pluginData = data;
				});
			});
			waitsFor(function() {return !!pluginData});
			runs(function() {
				expect(pluginData).toBe(expected);
			});
			
		}
		it("expect foo", function() {
			testIt("foo", true, false);
		});
		it("expect bar", function() {
			testIt("bar", false, true);
		});
		it("expect undefined", function() {
			testIt("undefined", false, false);
		});
	});
});

The test case uses the Jasmine unit test framework, but can easily be adapted to any framework.

The test fails on the second test case "expect bar" because the previously loaded string "foo" is returned.

The fix is provided in the pull request https://github.com/dojo/dojo/pull/96 on GitHub?.

There are two changes to dojo.js to fix this problem. The first is to clear the "load" property, in addition to the other properties already being cleared, of the module object in req.undef:

mix(module, {def:0, executed:0, i

Change History (6)

comment:1 Changed 5 years ago by chuckd

Ticket text was truncated on submission. Adding missing text here:

There are two changes to dojo.js to fix this problem. The first is to clear the "load" property, in addition to the other properties already being cleared, of the module object in req.undef:

mix(module, {def:0, executed:0, injected:0, node:0, load:0});

This is necessary so that the function promoteModuleToPlugin() will be called in injectPlugin():

if(plugin.executed === executed && !plugin.load){
	// executed the module not knowing it was a plugin
	promoteModuleToPlugin(plugin);
}

If promoteModuleToPlugin is not called, then the previously defined load function is reused after the module has been undef'ed.

The second fix is to add the !modules[mid].injected condition on the call to injectPlugin() in resolvePluginLoadQ():

if(!modules[mid] || !modules[mid].injected /*for require.undef*/){
	// create a new (the real) plugin resource and inject it normally now that the plugin is on board
	injectPlugin(modules[mid] = pluginResource);
} // else this was a duplicate request for the same (plugin, rid) for a nondynamic plugin

Without this added condition, the injectPlugin() function will not be called for an undef'ed plugin and the loader will time out waiting for the plugin to be defined.

With these fixes, the example test case passes.

Last edited 5 years ago by chuckd (previous) (diff)

comment:2 Changed 5 years ago by dylan

Milestone: tbd1.11
Priority: undecidedhigh

Thanks, it would definitely be better if this test was provided using Intern, as we're in the process of converting all of the tests to use Intern. Below is a start of that, though I'm not 100% certain about some of the details of your test, so it would be helpful if you could adapt it:

define([ 'intern!object', 'intern/dojo/Deferred', 'dojo/has', './foobarPlugin'
], function (registerSuite, Deferred, has, foobarPlugin) {
    var foo;
    registerSuite({
        name: 'mock plugin',
        setup: function () {
            var dfd = new Deferred();
            require.undef('./foobarPlugin');
            require.undef('./foobarPlugin!');
            require({ map: {
                'path': { 'dependency': 'mock' } // these 3 params need to be updated
            } });
            require([ './foobarPlugin' ], function (value) {
                foo = value;
                dfd.resolve();
            });
            return dfd.promise;
        },
        teardown: function () {
            require.undef('./foobarPlugin');
            require.undef('./foobarPlugin!');
            require({ map: { 'path': { 'dep': 'dep' } } }); // again, need to update these
        }
// and then add the real testcases here
    });
});

The point being that you'll want to make sure the setup and teardown methods are clean, so this can be included in the larger test suite...

comment:3 Changed 5 years ago by chuckd

Thanks Dylan. Here's the test case using Intern:

define([ 
	'require',
	'intern!object', 
	'intern/chai!assert', 
	'intern/dojo/Deferred',
	'dojo/has' 
], function(require, registerSuite, assert, Deferred, has) {

	function testIt(expected, fooValue, barValue) {
		has.add("foo", fooValue, true, true);
		has.add("bar", barValue, true, true);
		var dfd = new Deferred();
		require([ "./foobarPlugin!" ], function(data) {
			assert.strictEqual(data, expected);
			dfd.resolve();
		});
		return dfd;
	}

	registerSuite({
		name : 'foobarPlugin',
		setup : function() {
			require({async:true});	// only fails in async mode
		},
		beforeEach : function() {
			require.undef('./foobarPlugin!');
			require.undef('./foobarPlugin');
		},
		teardown : function() {
			require.undef('./foobarPlugin!');
			require.undef('./foobarPlugin');
		},
		expectFoo : function() {
			return testIt("foo", true, false);
		},
		expectBar : function() {
			return testIt("bar", false, true);
		},
		expectUndefined : function() {
			return testIt("undefined", false, false);
		}
	});

});
Last edited 5 years ago by chuckd (previous) (diff)

comment:4 Changed 5 years ago by chuckd

Updated intern test case to include setting async mode in setup function since undef only fails with plugin modules in async mode.

comment:5 Changed 4 years ago by dylan

Owner: set to dylan
Status: newassigned

comment:6 Changed 3 years ago by dylan

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.