Page MenuHomePhabricator

[OutputPage] Create a method to remove items from mModules
Closed, DeclinedPublic

Description

As mentioned in bug 23985 comment 3 and on http://www.mediawiki.org/wiki/Thread:Manual_talk:JavaScript_unit_testing/QUnit_/_TestSwarm_environment,_Version_2, we should implement a way for extensions (and core maybe as well) to easily undo an addition.

Either by specific name (public function removeModules), and/or for all (public function resetModules/clearModules).


Version: 1.20.x
Severity: normal

Details

Reference
bz29311

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 11:28 PM
bzimport set Reference to bz29311.

Added a note on bug 23985 comment 4 that this might be better implemented there with aliases or overrides.

I'm not sure how much other use there might be for a removeModule but I'd tend to be leery of it. A module could still be pulled in by a dependency on something else, for instance.

(In reply to comment #1)

I'm not sure how much other use there might be for a removeModule but I'd tend
to be leery of it. A module could still be pulled in by a dependency on
something else, for instance.

AFAIK Krinkle wants this for the QUnit special page, which needs to nuke all of mModules and only load test suites.

That should just need a reset then; or keeping it from having filled out with stuff in the first place.

It's probably wise to be able to run a lot of tests in-place in a real environment though; if modules conflict for instance, that's information tests should be turning up.

Only the lowest-level tests of the loader itself probably really require running in a clean slate environment.

john wrote:

Proposed fix

Created a patch with both the removeModules and resetModules functions as requested originally. Feel free to leave out the removeModules function if you feel it's unsafe.

Attached:

(In reply to comment #4)

Created attachment 8834 [details]
Proposed fix

Created a patch with both the removeModules and resetModules functions as
requested originally. Feel free to leave out the removeModules function if you
feel it's unsafe.

Looks good to me. I'd appreciate it if another committer could apply this, I don't have a clean working copy right now.

Attached:

(In reply to comment #5)

Looks good to me. I'd appreciate it if another committer could apply this, I
don't have a clean working copy right now.

r93751

EN.WP.ST47 wrote:

Apparently fixed in r93751

The original use case was to be able to surpress/filter out some modules form loading on a SpecialPage that needs isolation.

However it appears MediaWiki recently got support for that already by implementing "origin restrictions" (eg. when set to "CORE" resources by users, sites and extensions will not be loaded).

This removes the original use case from this feature request.

When looking back at what this method leaves, I'm not sure we should keep it. It can make debugging hard and also sets wrong expectation (ie. if a method adds a module to the output, it should actually get outputted and not unloaded by something else).

I defer to your judgement, Krinkle. Please feel free to revert.