Page MenuHomePhabricator

ResourceLoaderStartUpModule.php extendability
Closed, ResolvedPublic

Description

Author: mdale

Description:
Some minor changes to ResourceLoaderStartUpModule.php would help towards a path of extendability. ie the module set ( jQuery and MediaWiki ) should be a local method or variable so that extending classes can override "startup" library includes.


Version: unspecified
Severity: enhancement

Details

Reference
bz26805

Event Timeline

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

mdale wrote:

small patch to add hook to startup module to extend set of modules loaded at startup

This is needed if you want your "loaderScripts" to have access to any shared framework code.

attachment ResourceLoaderStartUpModule.php.patch ignored as obsolete

Could you explain in more detail what this is needed for?

mdale wrote:

This is needed for providing "core" functionality to module Loaders. ( There is no way to add a dependency to a loaderScripts ). When your loader Script is dependent on framework code, extending the startup modules is a way to express this dependency.

You can see how its used in TimedMediaHandler extension.

(In reply to comment #1)

Created attachment 8114 [details]
small patch to add hook to startup module to extend set of modules loaded at
startup

  • BEWARE: the round() call in the 'version' => setting has been removed, so this patch won't apply cleanly. Take care to resolve the conflict in a way that doesn't reintroduce the round() call
  • foreach( $modules as $moduleName){ doesn't conform to whitespace conventions

OK otherwise

attachment ResourceLoaderStartUpModule.php.patch ignored as obsolete

(In reply to comment #4)

OK otherwise

Two more things:

  • The hook should be documented in docs/hooks.txt
  • These docs should mention that this feature is to be used sparingly and for lightweight modules, as modules added through this hook will *always* be loaded, unconditionally (well except for people using ancient browsers)

mdale wrote:

updated patch with hooks documentation

Added hook documentation in updated patch.

Roan I still see round( max( on line 169 of ResourceLoaderStartUpModule as of version r82575

Attached:

(In reply to comment #6)

Created attachment 8243 [details]
updated patch with hooks documentation

Added hook documentation in updated patch.

Looks good now, except that "module scripts" in the hook documentation (last two words) should be "loader scripts". Feel free to commit after changing that.

Roan I still see round( max( on line 169 of ResourceLoaderStartUpModule as of
version r82575

Right, I apparently forgot to take that instance out. I will remove the round() call some time after you commit.

Attached: