This was done as a "clever" shortcut to make sure tests a little
but shorter, but also made them less consistent with normal code.
Remove this in favour of 'class' or 'factory' options as needed.
Also remove a bunch of unneeded register() calls.
The tests cover everything affected by this change.
Side fix - isFileModule should reject modules with 'factory'
the same way it rejected raw objects and non-FileModule 'class'
cases already. This is now covered by tests as well.
Bug: T222637
Change-Id: I3996317dbcd780cc6e0f82c84e769c08a3fc42bb
Being a raw module means that when it is requested from load.php with
"only=scripts" set, then the output is *not* wrapped in an
'mw.loader.implement' closure *and* there no 'mw.loader.state()' appendix.
Instead, it is served "raw".
Before 2018, the modules 'mediawiki' and 'jquery' were raw modules.
They were needed before the client could define 'mw.loader.implement', and
could never be valid dependencies. Module 'mediawiki' merged to 'startup',
and 'jquery' became a regular module (T192623). Based on the architecture
of modules being deliverable bundles, it doesn't make sense for there to
ever be raw modules again. Anything that 'startup' needs should be bundled
with it. Anything else is a regular module.
On top of that, we never actually needed this feature because specifying
the 'only=scripts' and 'raw=1' parameters does the same thing.
The only special bit about marking modules (not requests) as "raw" was that
it allowed the client to forget to specify "raw=1" and the server would
automatically omit the 'mw.loader.state()' appendix based on whether the
module is marked as raw. As of Ie4564ec8e26ad53f2, the two remaining use
cases for raw responses now specify the 'raw=1' request parameter, and we
can get rid of the "raw module" feature and all the complexity around it.
== Startup module
In the startup module there was an interesting use of isRaw() that has
little to do with the above. The "ATTENTION" warning there applies to the
startup module only, not raw modules in general. This is now fixed by
explicitly checking for StartupModule.
Above that warning, it talked about saving bytes, which was an optimisation
given that "raw" modules don't communicate with mw.loader, they also don't
need to be registered there because even if mw.loader would try to load
them, the server would never inform mw.loader about the module having
arrived. There are now no longer any such modules.
Bug: T201483
Change-Id: I8839036e7b2b76919b6cd3aa42ccfde4d1247899
The only remaining use of 'new ResourceLoader' is in tests, which have
been migrated in this commit to either passing the real config explicitly
(for integration tests), or by passing a HashConfig from a new
'getMinimalConfig' method which has only the keys required for the tests
to pass (e.g. avoid any ConfigExeption for unknown keys).
Also clean up some related code quality issues:
* Migrate wfScript() to $conf->get() so that the local Config is used,
instead of implicitly using global variables. This isn't deprecated for
MediaWiki generally, but done here to prepare ResourceLoader for becoming
a standalone library.
* Remove mocking of 'CacheEpoch' config, this is no longer used anywhere
in ResourceLoader.
* Change EmptyResourceLoader to use the minimal config by default and
remove code duplication by calling the parent.
Update the small number of uses that are integration tests, to explicitly
pass in the live config as needed. And for the one case that tests the
'startup' module, it no longer needs to register it manually given this
is part of ResourceLoader::__construct() by default.
Bug: T32956
Change-Id: I127346fd530fa66f205156e545758b1c29d0fac0
Mark the two basic integration tests as `@medium`.
These confirm that:
* The registration method throws on invalid names.
* The higher levels are able to support an empty string as name.
Change-Id: Ib8215408a28040986ae07f4b2421681bc885abad
Also restore the order of registrations as it was before
last week with 47422fabe2. (That is, core modules are registered
before extension modules, in case of conflicts with a warning, the
core one wins).
Bug: T32956
Change-Id: I3a50508178159dfc8e5db1e218a5e6d10e2d4b2a
This has always been an odd case, as indicicated by the cross-class
comment references, and the fact that its test cases are already
in ResourceLoaderTest.php, for convenience, as that's also where
the creation of 'module name strings' is done and tested.
Actually move it there instead of pretending it is there.
Change-Id: Ied9569436cc78704a5c1b75eeebb73f8631350f6
Reduce our reliance on static state and configuration, and
propagate more state in explicit ways, through context, and
request parameters.
OutputPage creates ResourceLoaderContext and ResourceLoaderClientHtml
based on the configuration (via ResourceLoader::inDebugMode).
Everything within those classes should not need to check it
again.
* ResourceLoaderClientHtml:
Already doesn't check MW config, but it's test was still
mocking it. Removed now, and confirmed that it passes both
with true and false. The individual test cases set
debug=true/false as needed already.
It's sets were previously relying on the accidental behaviour
that within a unit test, we don't serialise over HTTP, which
meant that a pure PHP boolean would survive. With the new
raw `=== 'true'` check, this no longer works. Set it as a
string explicitly instead, which is the only thing we support
outside unit tests as well.
* ResourceLoaderContext:
Remove fallback to MW config when 'debug' is unset.
This is never unset in practice given that all load.php
urls have it set by OutputPage based on ResourceLoader::inDebugMode.
This change means that manually constructed ad-hoc load.php
urls that are missing 'debug=' parameter, will now always be
read as debug=false. This was the default already, but could
previously be changed through wgResourceLoaderDebug.
When changing wgResourceLoaderDebug, everything will still have
debug=true as before. The only change is when constructing load.php
urls manually and explicitly not set it.
Bug: T32956
Change-Id: Ie3424be46e2b8311968f3068ca08ba6a1139224a
For config vars, add a virtual package file called 'config.js', and use
require( './config.js' ) in the module. For most data modules, add a
virtual package file called 'data.js', use require( './data.js' ) in the
module. Where needed, add wrapper files that put the data in the
relevant global object and in module.exports.
LanguageDataModule is the only special-purpose module not being removed
in this commit, because it uses languageScripts, and those are not
compatible with packageFiles (yet).
Also merge mediawiki.ForeignStructuredUpload.config into
mediawiki.ForeignStructuredUpload, since that was the only thing that
used it.
Change-Id: I203d4e3ecdeeeb16729eba2dcf40d11a41d2e582
Package files are files that are part of a module, but are not
immediately executed when the module executes. Instead, they are
lazy-excecuted when require() is called on them. Package files can be
scripts (JS) or data (JSON), and can be real files on the file system,
or virtual files generated by a callback.
Using virtual data files, server-side data and config variables can be
bundled with a module. Support for file-based require() allows us to
import npm modules into ResourceLoader more easily.
The require function passed to each script execution context, which was
previously a reference to the global mw.loader.require() function, is
changed to one that is scoped to the module and the file being executed.
This is needed to support relative paths: require( '../foo.js' ) can
mean a different file depending on the path of the calling file.
The results of require()ing each file (i.e. the value of module.exports
after executing it) are stored, and calling require() on the same file a
second time won't execute it again, but will return the stored value.
Miscellaneous changes:
- Add XmlJsCode::encodeObject(), which combines an associative array of
XmlJsCode objects into one larger XmlJsCode object. This is needed for
encoding the packageFiles parameter in mw.loader.implement() calls.
Bug: T133462
Change-Id: I78cc86e626de0720397718cd2bed8ed279579112
This might hint at an edge-case in the PHP CodeSniffer sniff that should
detect if methods are separated by a single empty line. Feel free to
investigate. I, personally, can't invest more time in this than
suggesting this quick fix.
Change-Id: Ib3c60eac76f255b4fe929f7933de256222716576
* There is only a single non-test caller to this method in the entire
codebase, and no callers elsewhere (Wikimedia Git, Codesearch).
It's only used with an array, so remove the other unused code paths,
and mark it internal (to my knowledge, nothing ever used it outside
RL in the past, either).
* Add test coverage for the module indexing logic.
Change-Id: I9e0f95416d5b2fdd87323288231ee6d8c85d88e7
Follows-up b7b84d55d4, which embedded the whole of the three 'mediawiki'
JS files inside of startup.js, but did so inside of the pre-existing
closure that was there.
This adds some overhead we don't get value of, so best to remove
it and embed it as a sibling of startup.js in the same top-level
scope.
In local testing (Chrome stable, CPU 1/6th) this reduced startup
run-time from 73-78ms to 63-65ms.
Also:
* Declare isCompatible() as a normal function.
Disable the implicit-globals warning given this isn't a regular module,
this file is intentionally in the global scope.
* Unfold the private startUp() function to its call site.
Change-Id: Ida51ab20898c9e4ae6cbf7ad2968d88d824a1483
The PHP interface still supports it without deprecation, but there
is no need for the client to support it given it is only ever called
from the startup module, which always passes it an object.
Follows-up 6815e35575, which did the same for `state(key, val)`.
Also simplify the implementation to avoid extra calls and optimise
for the only signature.
Mark the method as @private, and change the existing use of
@protected to @private (doesn't really make sense in JavaScript).
Change-Id: I24786f90bcfb256b2e7c37f7760bba1a5e399443
The use of global variables was deprecated in favour of
ResourceLoaderModule::getLessVars() on a per-module basis.
Also moved testLessFileCompilation case to the appropiate file as it
covers ResourceLoaderFileModule.php, not ResourceLoader.php.
Bug: T140804
Depends-On: Ib1b2808df2384473bfac47f53a5d25d7c9bbca2b
Depends-On: I96047f69d01c4736306df2719267e6347daf556f
Change-Id: If708087c85c80355c7e78f1768529b5f2e16ed07
- Cover case of simple module load.
The bulk of this use case is already covered by a lower-level
test for makeModuleResponse(). The added case here exists
to cover the wrapper method, ResourceLoader::respond().
- Cover logic for catching and logging internal errors.
Change-Id: I4315bb00137ff80ee2b790c6b4d4b5fbd93f6bc1
The last remaining users of this feature were MobileFrontend and Minerva,
which have been migrated to direct imports.
Bug: T140807
Change-Id: I1a66a2ad314bde332297798520e5ec3e0e3d4c9b
Adds coverage for line 1049-1056.
Also, follow-up 6292d54dff by simpliying the regex by using /s modifier to
enable PCRE_DOTALL which includes matching of new lines.
Change-Id: Icec34dfe107d418951b3d155234295c79410ffaa
This prevents cache churn when the wiki-global LESS variables vary
between wikis because the cache key is used as a "global" instead of
db-local. This is good for the common case, but should still explicitly
vary if the vars differ between wikis.
Bug: T191937
Change-Id: If12fd07a7062792205384150d6f5fd9a83f996cc
This was previously covered implicitly by an unrelated test.
Change that test (dependency.less) to use ../ to access the file
directly so that that test case is only about tracking dependencies
and testing the parser.
Then, add a second case that tests the use of import dirs.
Bug: T140807
Change-Id: Ie85abffe313922c03b3e146422f36b1d6a79743d
Verify that calling register() twice does not throw, but warns,
and that the last registration wins.
This behaviour was actually surprising to me because it used to
throw, and I'd assume that when we added the warning, the behaviour
would go from fatal to non-fatal, but keep that the last one is
at fault/unsupported.
Perhaps b1ea0612 / d3e3bcfd6 (T116628) should've added a return
statement. Oh well, we can consider changing that later, but at
least test for it.
Change-Id: I955132868146ea5bf88c9b9e648c84d8196cb1f9
It previously depended on Xml.php for Xml::encodeJsCall, which
in turn depends on FormatJson. Other Xml.php methods also depend
on Sanitizer, but that was carefully not triggered.
Avoid both of these by simply constructing the string inline,
and using json_encode() directly for the one variable parameter.
Also, fix the names of one of the ResourceLoaderTest cases for
dotless module names. It claims to have been added to cover a
regression fixed in r88706, but that commit seems rather unrelated.
A bit of searching revealed as the relevant commit instead.
Bug: T188076
Change-Id: I04851d0355227f3a6b79b8d41a51d4beadce0e80
We currently have several counts and timings of individual pieces
of ResourceLoader backend logic (minification, module building, ..)
but no measure of the response overall.
This responseTime metric will effectively provide both a timing
measure as well as a backend request count.
Bug: T178350
Change-Id: I625a5eb90f5a4ea90aebf9292dfda0f1c5ae4f2e
ResourceLoaderModule objects gain a new method getPreloadLinks() which
returns an array with the meta data required to build a Link rel=preload
header according to the current draft for W3C Preload.
<https://w3c.github.io/preload/>
Another implementation of this is already in use in OutputPage for
preloading the logo image.
This array is formatted by the ResourceLoaderModule::getHeaders method,
which is implemented as "final" at this time, thus restricting use to
the Link rel=preload header.
Headers are exposed and process-cached, like all other content
(scripts, styles, etc.), through ResourceLoaderModule::getModuleContent,
and aggregated by ResoureLoader::makeModuleResponse.
I had hoped for the getPreloadLinks to be stateless (not vary on $context).
Whether something should be preloaded and what, should not vary on the
skin or language. However, while that conceptually holds true, the exact
url for any given resource may still vary. Even the main use case for this
feature (T164299, preloading base modules request) require $context to pass
down skin and lang to the load.php url.
Add full test coverage and example documentation.
Bug: T164299
Change-Id: I2bfe0796ceaa0c82579c501f5b10e931f2175681
This fixes two bugs:
* 1) When two modules are requested, and the first one ends with ";"
inside a comment, the second module might become part of
that comment.
* 2) A request with script=only where the requested module content
ends in a statement without ";", but has a comment after it
that does ends with a semicolon, then in debug=false, mw.loader.state()
would be appended directly after the semicolon-less statement because
the check is performed before minification.
Previously:
script> foo()
script> // bar();
states> mw.loader.state( {} );
Became (minified separately):
script> foo()
states> mw.loader.state({});
Became (concatenated)
> foo()mw.loader.state();
Which is invalid code.
Both of these are now fixed.
Bug: T162719
Change-Id: Ic8114c46ce232f5869400eaa40d3027003550533
The used phpcs has a bug, so the version 0.9.0 could not be enforced at the moment.
Will be fixed in next version, see T167168
Changed:
- Remove duplicate newline at end of file
- Add space between function and ( for closures
- and -> &&, or -> ||
Change-Id: I4172fb08861729bccd55aecbd07e029e2638d311
This should work the same way as registering API modules via a factory callback.
Point in case: Ifb8611473a971 could avoid global state using this mechanism.
Change-Id: Ifbf29006141ce2a2dff42efa352f406502a06bc6
Replaces \TestingAccessWrapper (defined in core) with
\Wikimedia\TestingAccessWrapper (defined in the composer package
wikimedia/testing-access-wrapper).
See https://gerrit.wikimedia.org/r/#/q/topic:librarize-testing-access-wrapper
for downstream patches.
The core version of the class is kept around for a while to avoid
circular dependency problems.
Bug: T163434
Change-Id: I52cc257e593da3d6c3b01a909e554a950225aec8
* Missing cases for StartupModule::getModuleRegistrations
(now 100% covered)
- Raw modules are omitted from the manifest.
E.g. The base modules ('jquery', 'mediawiki') are raw modules
that we don't register client side (they can't load themselves).
- Exceptions from getVersionHash() are caught.
- Oversized versions are re-hashed.
* Missing cases for ResourceLoader::makeLoaderRegisterScript.
(now 100% covered)
* Missing cases for ResourceLoader::getModule.
(now 100% covered)
Change-Id: If9717a48195fc6ae776da5d0e86f323d7f60426d
> 1) ResourceLoaderTest::testMakeModuleResponseError
> Failed asserting that '[e08c982d974548127cb5d7ce] Fatal exception of type Exception'
> matches PCRE pattern "/Ferry not found/".
> .../ResourceLoaderTest.php:519
This happened on Travis CI, because ResourceLoader::formatException() behaves
differently based on $wgShowExceptionDetails. Which is enabled in Vagrant
and Jenkins, but disabled by default (and thus in Travis CI builds).
Bug: T75176
Change-Id: If15dd03213703b7b6ff899cad5e5569e2515b378
When getScript (or some other method used in a module response)
throws an error, only that module fails (by outputting mw.loader.state
instead of mw.loader.implement). Other modules will work.
This has always been the case and is working fine. For example,
"load.php?modules=foo|bar", where 'foo' throws, will return:
```js
/* exception message: .. */
mw.loader.implement('bar', ..)
mw.loader.state('foo', 'error')
```
The problem, however, is that during the generation of the startup
module, we iterate over all other modules. In 2011, the
getVersionHash method (then: getModifiedTime) was fairly simple
and unlikely to throw errors.
Nowadays, some modules use enableModuleContentVersion which will
involve the same code path as for regular module responses.
The try/catch in ResourceLoader::makeModuleResponse() suffices
for the case of loading modules other than startup. But when
loading the startup module, and an exception happens in getVersionHash,
then the entire startup response is replaced with an exception comment.
Example case:
* A file not existing for a FileModule subclass that uses
enableModuleContentVersion.
* A database error from a data module, like CiteDataModule or
CNChoiceData.
Changes:
* Ensure E-Tag is still useful while an error happens in production
because we respond with 200 OK and one error isn't the same as
another.
Fixed by try/catch in getCombinedVersion.
* Ensure start manifest isn't disrupted by one broken module.
Fixed by try/catch in StartupModule::getModuleRegistrations().
Tests:
* testMakeModuleResponseError: The case that already worked fined.
* testMakeModuleResponseStartupError: The case fixed in this commit.
* testGetCombinedVersion: The case fixed in this commit for E-Tag.
Bug: T152266
Change-Id: Ice4ede5ea594bf3fa591134bc9382bd9c24e2f39
It's not explicitly supported anywhere, but I don't see a point in explicitly
disallowing it. Add unit tests to verify that this works.
Bug: T28804
Change-Id: I876ac43885bb27da54ef6e59b6416868ff636b84
* Keep this out of makeLoaderImplementScript() to keep it more generic
and to simplify future refactoring.
* Remove now-broken test case that asserted that the output varies
by global debug mode.
* Make the test responsible for wrapping in XmlJsCode. Previously
this magically happened because the module name was "user" in the
last test case.
* Make makeLoaderImplementScript protected. It's not used anywhere
outside ResourceLoader and we should keep it that way.
Test plan:
* Verify output unchanged:
- load.php?modules=user&only=scripts&user=Admin (raw code)
- load.php?modules=user&user=Admin (implement with unwrapped string)
- load.php?modules=jquery.client (implement with closure)
Change-Id: I527d01926fb6e4ce68c931695d830cdb9ceb608c
* Fix up one last use of global config vars in this class.
Other places in this class already used $rl->getConfig().
This way we don't inherit all of MediaWikiTestCase.
* Add unit tests covering all of ResourceLoaderContext
except expandModuleNames and getImageObj (tested in better
places already with the right @covers).
* Increase coverage for expandModuleNames(), add missing case
of when modules are not in alphabetical order.
Change-Id: Id19b084d37a6c3a77b36e03509adffb6b156fee1
* Fix signature of makeLoaderSourcesScript() to match
the change in behaviour since e103ba265.
* Consistently order providers before the test.
* Simplify testRegisterValid() and remove needless @depends.
* Remove unused private method stripNoflip().
Coverage:
* Expand test coverage for register().
* Add tests for getModuleNames().
* Add tests for getModule().
* Expand test coverage for addSource().
(case of invalid array)
* Expand test coverage for makeLoaderImplementScript().
(case of unwrapped user script, and case of invalid scripts)
* Add tests for makeLoaderSourcesScript().
Change-Id: Ibca3e486fcd3664f171f135327a0f340ee6da9ee
* Send 'module' and 'require' parameters to module closures.
This depends on Ia925844cc22f143 being deployed one cycle earlier.
* Patch Moment and OOjs to ensure these libraries continue to expose
their module as globals as well. AMD/UMD-compatible libraries
only expose a global *OR* an export, not both. We need both
for back-compat.
* Update pluralRuleParser to make use of module export to allow
usage via require().
To test, check out the patch and run:
> mw.loader.load('moment');
> mw.loader.require('moment')()
> mw.loader.require('moment')('2011-04-01').fromNow()
Bug: T108655
Change-Id: Idbd054880ee70d659ec760aef8fcb38d0704a394
Deprecated in 1.24, for reasons explained in a0c41ae39d. I don't see any
usage in core or extensions.
Change-Id: I46f9e04ae633e7ff1ee112b652e1865731172f1f
Follows-up ebeb29723, 1f393b6da, 0e719ce23.
Also:
* Add tests for ResourceLoader::makeLoaderImplementScript().
* Apply ResourceLoader::trimArray to makeLoaderImplementScript (new in c0c221bf).
This commit changes the load.php response to omit empty parameters.
These parameters were required until recently. The client has been
updated (1f393b6da and 0e719ce23) to make these optional, thus supporting
both the old server format and the change this commit makes
Clients with a tab open from before 0e719ce23 are naturally not
compatible with load.php responses from after this commit. Ensure
this is deployed several days after 0e719ce23 to reduce race
conditions of this nature.
(This is a re-submitted version of 4ce0c0da4)
Bug: T88879
Change-Id: I9e998261ee9b0b745e3339bc3493755c0cb04b6a
