This reverts commit e08ea8ccb9.
Reason for revert: Breaks Phan in extensions, and as far as I’m aware,
this change isn’t urgently needed for anything, so the simplest fix is
to revert it again for now. After PHP 7.4 it should be safer to try this
again (we hopefully won’t need the two “hack” classes by then).
Bug: T308443
Change-Id: Iff3318cbf97a67f821f78e60da62a583f63e389e
Move ResourceLoader classes to their own namespace. Strip the
"ResourceLoader" prefix from all except ResourceLoader and
ResourceLoaderContext.
Move the tests by analogy.
I used a namespace alias "RL" in some callers since RL\Module is less
ambiguous at the call site than just "Module".
I did not address DependencyStore which continues to have a non-standard
location and namespace.
Change-Id: I92998ae6a82e0b935c13e02a183e7c324fa410a3
Now largely automated:
VARS=$(grep -o "'[A-Za-z0-9_]*'" includes/MainConfigNames.php | \
tr "\n" '|' | sed "s/|$/\n/;s/'//g")
sed -i -E "s/'($VARS)'/MainConfigNames::\1/g" \
$(grep -ERIl "'($VARS)'" includes/)
Then git add -p with lots of error-prone manual checking. Then
semi-manually add all the necessary "use" lines:
vim $(grep -L 'use MediaWiki\\MainConfigNames;' \
$(git diff --cached --name-only --diff-filter=M HEAD^))
I didn't bother fixing lines that were over 100 characters unless they
were over 120 and triggered phpcs.
Bug: T305805
Change-Id: I74e0ab511abecb276717ad4276a124760a268147
On special pages where disallowUserJs() is used, or when setting
safemode=1 by hand to index.php, disable mw.loader.store.
OutputPage already normalizes disallowUserJs() to safemode=1 as
passed on to ResourceLoaderClientHtml and ResourceLoaderContext.
Use it in the startup mode to disable this feature.
Invert the internal substitution variable to avoid confusing
double negatives, and to match the config name.
Bug: T145498
Change-Id: I2fdba968b4703ec8826bdaf91340bdead058f02b
Registering fewer modules reduces the size of the startup module.
The current skin is read from the skin parameter to the load.php
request. Modules can opt-in by specifying skins in which they are
relevant, by overriding getSkins() of ResourceLoaderModule.
Bug: T253582
Bug: T236603
Change-Id: I08a1a59b4eab90d380ca8f874bb6dbba2f399590
Don't catch and discard exceptions from the RequestTimeout library,
except when the exception is properly handled and the code seems to be
trying to wrap things up.
In most cases the exception is rethrown. Ideally it should instead be
done by narrowing the catch, and this was feasible in a few cases. But
sometimes the exception being caught is an instance of the base class
(notably DateTime::__construct()). Often Exception is the root of the
hierarchy of exceptions being thrown and so is the obvious catch-all.
Notes on specific callers:
* In the case of ResourceLoader::respond(), exceptions were caught for API
correctness, but processing continued. I added an outer try block for
timeout handling so that termination would be more prompt.
* In LCStoreCDB the Exception being caught was Cdb\Exception not
\Exception. I added an alias to avoid confusion.
* In ImageGallery I added a special exception class.
* In Message::__toString() the rationale for catching disappears
in PHP 7.4.0+, so I added a PHP version check.
* In PoolCounterRedis, let the shutdown function do its thing, but
rethrow the exception for logging.
Change-Id: I4c3770b9efc76a1ce42ed9f59329c36de04d657c
This change exposes the requiresES6 flag to the public API,
specifically via getState() by not registering modules that
aren't available / can't be loaded in the context.
This actually simplifies the code a fair bit, and partially reverts
the added logic from I45670c910ff12.
Bug: T299677
Change-Id: I96a03796628a74ace93579d45a582711400c09c1
== What ==
Change debug mode 2 to behave more like production mode:
* use module scope (no longer global scope).
* load modules concurrently (no longer each module serially).
* bundle files (no longer each file separately).
What remains different in debug=2 from production mode:
* disable minification.
* disable batching (one module per request).
== How ==
* Limit the old logic (getScriptURLsForDebug) to just legacy debug.
* Set maxQueryLength=0 for non-legacy debug, to ensure each module
still gets its own dedicated request for easy debugging, and to
get concurrency to make more optimal use of server and browser
capacity.
This does not effect package file modules much, as those already
worked in this way. The only difference for package file modules
is that they now load faster (see below) by eliminating the
in-between request.
== Alternative approach ==
An alternative approach, which I considered, is to modify
Module::buildContent(), around where we currently call
getScriptURLsForDebug for DEBUG_LEGACY, and add a conditional branch
for DEBUG_MAIN which would always return an array with a single URL,
to `load.php?modules=:name`. Much like getScriptURLsForDebug does by
default, but without the legacy-specific overrides to that method from
e.g. FileModule.
I decided against this because the mw.loader client handles such
script-arrays in a global FIFO fashion, tailored for legacy debug mode
where it crucial to only serially queue>load>execute one script file
of one module at any given time (because the raw files can't have a
"mw.loader.implement" closure and thus execute immediately on arrival,
with no other coordination for file order and module dependency order).
This would make debug=2 slow, possibly slower than debug=1 since in
debug=1 at least we consolidate most PHP roundtrips in a single batch,
and most other scripts can be served quickly as static file by Apache.
By letting the client act like it does for production mode, and
proactively split its requests, we get a few benefits compared to
this alternative approach:
* Fewer requests and shorter request dependency chain.
There is no in-between request for the "page module batch" that fans
out to individual module reqs. Instead, the client makes those reqs
directly.
* All module requests are discovered and queued with the browser in
one go, letting the server handle them as quickly as it can.
In production, probably all in parallel. Locally, mediawiki-docker
seems to handle about 6 at time (this depite having 10 php-fpm
proccess). I suspect that maybe due to a poor interactions between
HTTP1 connection reuse and keep-alive timeouts, or perhaps unneeded
session locks with sqlite.
* The browser can spend time parsing/compiling other requests at the
same time as one of them executes.
* No additional client-side logic.
* No increase in client payload.
Bug: T85805
Change-Id: I232310eb624e0204484ec9f3d715d5b6b8532fe8
In all cases where the client registry is used, the `noscript` module
is inapplicable. Additionally, make it illegal to have a module
include the 'noscript' module as a dependency.
Bug: T291735
Change-Id: I5846f2c19b003989b56b12628b385fd37049483b
=== Why
* More speed
In debug mode, the server should regenerate the startup manifest
on each page view to ensure immediate effect of changes. But,
this also means more version recomputation work on the server.
For most modules, this was already quite fast on repeat views
because of OS-level file caches, and our file-hash caches and
LESS compile caches in php-apcu from ResourceLoader.
But, this makes it even faster.
* Better integration with browser devtools.
Breakpoints stay more consistently across browsers when the
URL stays the same even after you have changed the file and
reloaded the page. For static files, I believe most browsers ignore
query parameters. But for package files that come from load.php,
this was harder for browsers to guess correctly which old script URL
is logically replaced by a different one on the next page view.
=== How
Change Module::getVersionHash to return empty strings in debug mode.
I considered approaching this from StartupModule::getModuleRegistrations
instead to make the change apply only to the client-side manifest.
I decided against this because we have other calls to getVersionHash
on the server-side (such as for E-Tag calculation, and formatting
cross-wiki URLs) which would then not match the version queries that
mw.loader formats in debug mode.
Also, those calls would still be incurring some the avoidable costs.
=== Notes
* The two test cases for verifying the graceful fallback in production
if version hash computations throw an exception, were moved to a
non-debug test case as no longer happen now during the debug
(unminified) test cases.
* Avoid "PHP Notice: Undefined offset 0" in testMakeModuleResponseStartupError
by adding a fallback to empty string so that if the test fails,
it fails in a more useful way instead of aborting with this error
before the assertion happens. (Since PHPUnit generally stops on the
first error.)
* In practice, there are still "version" query parameters and E-Tag
headers in debug mode. These are not module versions, but URL
"combined versions" crafted by getCombinedVersion() in JS and PHP.
These return the constant "ztntf" in debug mode, which is the hash
of an empty string. We could alter these methods to special-case
when all inputs are and join to a still-empty string, or maybe we
just leave them be. I've done the latter for now.
Bug: T235672
Bug: T85805
Change-Id: I0e63eef4f85b13089a0aa3806a5b6f821d527a92
This is in preparation for making all version hashes the empty string
in debug mode, which will make things faster to work with, but also
helps solve T235672 in an easy way client-side without having to
add additional complexity to the mw.loader client that is specific
to debug mode.
Bug: T235672
Change-Id: I43204f22dfbcf5d236b35adc5b35df3da8021bad
When boolean values are minified, they get replaced by `!0` and
`!1` for true and false respectively. Since the mw.loader code wants
to check if the store is *disabled*, it uses `!$VARS.storeEnabled`,
which gets converted to either `!!0` or `!!1`. Inverting the variable
passed to the JavaScript code saves an extra character from the
response size, and makes the code a bit clearer.
The next version of the Minify library removes the conversion of
true and false to `!0` and `!1`, meaning that `!$VARS.storeEnabled`
will either be `!true` or `!false` - after this patch it become `false`
or `true` instead, resulting in the same number of characters in the
first case but two fewer in the second.
This should have no impact on the functionality.
Change-Id: I0cc3de3d2d43fc5ead65b471bc246dcd5bc4ae1e
The line exposing the defineFallbacks() method for tests,
`mw.redefineFallbacksForTest = window.QUnit && defineFallbacks;`,
is run unconditionally on all requests. However, this is only used
for testing in via Special:JavaScriptTest, which is only available
if $wgEnableJavaScriptTest is true, which for production sites is
not the case.* Thus, we don't even need to include the line. Replace
it with a $CODE substitution ResourceLoaderStartUpModule.
Additionally, move it a bit further up in the code to be grouped with
where defineFallbacks() is defined.
Should be a no-op in terms of functionality.
* Technically, we only need it if JavaScript tests are enabled,
and we are on Special:JavaScriptTest, but that doesn't seem
easy to check and isn't as important of an optimization.
Change-Id: I2f0d9443a118e713cc7016c4bc0102fdd2071ec0
Remove primary IIFE when setting mw.loader, which reduces
indentation by an extra level. Futher cleanup will be done
to the code for mw.loader.store in a follow-up.
The fnv132() function, which is only used in the loader, was
among the extra bits that were moved.
Should have no effect on the functionality.
Change-Id: Ie475879727e4170eb47f3fc78048379c7190844d
There is a fallback in Module->getConfig() to the global services
container. This is not meant to be used in practice, but there were
two places where this was missing: WebInstallerOutput, and various
PHPUnit tests.
* Add missing setConfig() to WebInstallerOutput.
* Add missing setConfig() to various tests. Most tests were already
doing this correctly and using the standard mock from
ResourceLoaderTestCase. Upon switching the last few tests as well,
I uncovered various errors due to the mock missing some settings
that the tested code uses, so these have been added now to
ResourceLoaderTestCase.
Bug: T277728
Change-Id: I44f16ec4e00423fb6f641e58fffc1d40e4571f01
Instead of monkey-patching mw.log from a conditionally loaded file,
patch it in-place using a $CODE substitution, as we already do for
other things.
Per <https://caniuse.com/console-basic>, conditional use of console is
no longer needed. Basically, only in IE 6-9 was it needed to check
whether window.console exists. The API exists in all Grade A browsers
now. It may still be a no-op in certain cases (like Mobile Safari
unless developer mode is enabled), but that's fine and was already
the case before since it satisfied the conditional.
For production mode, use of console.warn and console.error remain
conditional on their existence as before, though I've renamed the
local variable 'console' to 'con' to avoid shadowing the global,
and we now only check for the specific method, the object must exist.
Change-Id: Ied4d08c554936b0d86ff4a5fae749c9a2110badf
Modules that set "es6": true in their module definition will error when
a non-ES6 client tries to load them.
To detect ES6 support, this looks for native Promise support,
RegExp.prototype.flags, and non-BMP characters in variable names. All
browsers that lack full ES6 support fail at least one of those checks.
To flag modules as requiring ES6, this adds a ! to the end of their
version string. This takes up much less space than adding another
register() parameter (which would have to be at the end). It's hacky,
but we expect this feature to be relatively temporary, until we require
ES6 for running any JS at all (probably in about a year).
For distinguishing different types of errors thrown from
sortDependencies(), use e.name. We can't subclass Error properly because
that requires ES6.
Bug: T272104
Change-Id: I45670c910ff12eb422ae54c9fcf372e45c7b2bf1
* Add a void return hint to methods that are not meant to return
anything. This helps catch accidental return statements in the
future, lets Phan better understand how methods are meant to be
used, and might also allow PHP to better optimise the compiled
code form (speculation).
I did not, however, add it to publicly extended methods as that
might mess with strict mode.
* Remove the internal getResourceLoader() method from MessageBlobStore.
This has been redundant since 3edaa0b37c.
The method was protected, and not considered stable to subclass
for extensions. Hence not a breaking change.
* Add Throwable typehint to formatException() and friends.
This is the narrowest one I could add given that the methods
called from here already enforce the same typehint.
Update the doc to match for now. There isn't a reason right now
to limit this only to Exception, and given this is the method
and not the catch statement itself, does not change behaviour.
* Remove unused ResourceLoader->getHookContainer().
Added within 1.35 cycle, safe to remove.
* Remove unexpected `@since` for `@internal` getHookRunner().
* Remove redundant `@internal` from ResourceLoaderMwUrlModule
methods, which itself is already `@internal`.
Change-Id: I68d33ff6feca7ef95282a7ff03eb9332adfde31c
Follows-up 3ac385a0c3. This was generated by the installer at some
point and we've received two user reports of someone being caught by
this. We don't need to support "unlimited" anymore, but at least make it
do something more sensible, like using the default of 2000.
Previously, it was effectively treating the -1 like 0,
which was causing "debug mode"-like behaviour for end users.
Bug: T251789
Change-Id: I483d5312e6fa25a0b00bb6173ed01eeb99ad42aa
=== Observe the bug
1. Run Special:JavaScriptTest
(add ?module=mediawiki.loader to run only the relevant tests)
2. In the Network panel, check the JS requests to load.mock.php?…
3. Without this patch, they are like:
"load.mock.php?1234?lang=en&modules=…&…"
With this patch, they are like:
"load.mock.php?lang=en&modules=…&…"
The question mark is only valid as the start of the query string,
not as divider between them. This means without this patch, the
"lang" parameter is simply ignored because it becomes part
of the key "1234?lang" with value "en".
=== What
The mock server doesn't do anything with "lang". And given that
RL sorts its query parameters for optimum cache-hit rate, the
corrupted parameter is always "lang", as its sorts before
"module" or "version", which our mock server does utilize.
As part of server-side compression of the startup module (d13e5b75),
we filter redundant base parameters that match the default. For
RLContext, this is `{ debug: false, lang: qqx, skin: fallback }`.
As such, if one were to mock the localisation backend with
uselang=qqx internally, the "lang" parameter will not need to be
sent, and thus the above bug will start corrupting the "modules"
paramater instead, which our test suite correctly detects as being
very badly broken.
=== Why
mediawiki.loader.test.js used QUnit.fixurl() as paranoid way to
avoid accidental caching. This blindly adds "?<random>" to the
url. Upstream QUnit assumes the URL will be a simple file on disk,
not expecting existing query parameters.
=== Fix
* Removing the call to QUnit.fixurl(). It was set by me years ago.
But, there is no reason to believe a browser would cache this
anyway. Plus, the file hardly ever changes. Just in case,
set a no-cache header on the server side instead.
* Relatedly, the export of $VARS.reqBase is an associative array in
PHP and becomes an object in JSON. Make sure this works even if
the PHP array is empty, by casting to an object. Otherwise, it
becomes `[]` instead of `{}` given an PHP php array is ambiguous
in terms of whether it is meant as hashtable or list.
Bug: T250045
Change-Id: I3b8ff427577af9df3f1c26500ecf3646973ad34c
This data isn't needed for startup, and we can shave off a few K from
startup by moving it to mediawiki.base instead.
It was requested that this be done as a "package file", which
necessitated some other minor structural changes to mediawiki.base as
well.
Bug: T235350
Change-Id: I525a5203533089d5a542f83a847be58a10cb6319
It's only=scripts, not only=script. As written, following the
instructions in the comment that StartUpModule returns when you're using
it wrong doesn't work, this fixes that.
Change-Id: Ib0cb2ce18dbc446dc3c5f3c670b4c29064d000d9
This change allows to use the context in the functions.
The following internal static functions from ResourceLoader get now a
reference to the ResourceLoaderContext object:
* makeLoaderImplementScript
* makeLoaderStateScript
* makeLoaderRegisterScript
* makeLoaderSourcesScript
ResouceLoader::encodeJsonForScript is duplicated to
ResourceLoaderContext::encodeJson loading the debug mode from context.
ResourceLoader::encodeJsonForScript is kept for other usages without
context.
The debug mode is loaded from $context->getDebug() instead of from
ResourceLoader::inDebugMode(). This does not support to enable the debug
mode by setting the cookie 'resourceLoaderDebug' or the configuration
variable wgResourceLoaderDebug. Only the URL parameter debug=true
enables the debug mode. This should be sufficient for the subsequent
ResourceLoader requests. The tests don't need the global variable
wgResourceLoaderDebug anymore. The initial ResourceLoader context in
OutputPage still uses ResourceLoader::inDebugMode() with cookie and
global configuration variable.
This change adds the parameter $context with a ResourceLoaderContext
object to ResourceLoaderModule::getDeprecationInformation and deprecates
omitting the parameter. Ifa1a3bb56b731b83864022a358916c6aca5d7c10
updates this in extension ExtJSBase.
Bug: T229311
Change-Id: I5341f18625209446a6d006f60244990f65530319
* Add license header where missing.
* Add missing `@since` (1.17 for most classes), except
ResourceLoaderLessVarFileModule since 1.32 (1bc62c548c).
* Remove duplicate file-level description for class-only files,
merge with the class description instead.
* Remove my own `@author` annotation from one file.
* Mark core's own FileModule subclasses as `@internal`, except
for the following which we support use of in extensions:
ResourceLoaderLessVarFileModule,
ResourceLoaderOOUIIconPackModule, and
ResourceLoaderWikiModule.
Change-Id: I336af2e4ccdbe2512594e8861b72628d24194e41
This was previously just added by Echo, but it's generally useful.
We should probably deprecate wgDBname, because wgWikiID is a better wiki
identifier that also works when two wikis have different table prefixes
in the same database, but that'll take some work because a number of
things rely on it right now (including ResourceLoader itself, for its
localStorage keys).
Change-Id: I4d289267991f1f9a8e0710ec6ee5a2131306c510
In a nut shell:
* We very often (52% of modules on enwiki) pad the hash with a zero,
which means the amount of bits we currently compute already fit in
6 characters already for most modules. For some modules (3%) we
even padded two zeroes.
* For the (now documented) use cases, the space of 78 Giga
(78 billion, or 78 milliard) seems more than we need. The space of
60 million should be enough.
This follows-up dfd046412f from 2016, which previously shortened the hash
down from 8 chars of base 64 (or 12 chars of hex) to 7 chars of base 32.
Before that change, the space was 281 Tera (64^8, or 16^12).
For more details see the added inline comment for ResourceLoader::makeHash,
and also the data at <https://phabricator.wikimedia.org/T229245>.
Bug: T229245
Change-Id: I9ad11772a33b3a44cb625275b1d7353e1393ee49
I added this as wrapper for the private 'getBaseModules' method
in commit dec800968e for "use by SpecialJavaScriptTest", but
I never did. And it doesn't need it so.. let's remove that.
Change-Id: Ia301bd1c1f3a15098b9e6cd2934dd8e80879dd50
This isn't needed because the startup module validates this already.
The vast majority of modules are FileModule instances which can't be invalid,
because a separate test asserts that class already. This test existed for
validating the format of version hashes returned by a theoretical Module
sub class in an extension that (badly) overrides the getVersionHash method.
As of writing, no extension overrides that method. And more importantly,
the startup module already validates this at run-time, and logs a warning.
This commit turns that into an exception, which would get logged in a way
that Jenkins will fail the build if encountered.
This structure test, which computed the response for all registered modules,
previously took 3-5 seconds in CI.
Bug: T225730
Change-Id: Id2e37434b0ccd95dd2279f04e2230e9c06b09ccb
Before this change, the startup module responds as follows:
```lang=js
... contents of startup/mediawiki.js:
...... mw.config = null;
...
... contents of startup/startup.js:
...... mw.config = new mw.Map( bool );
```
After this change:
```lang=js
... contents of startup/mediawiki.js:
...... mw.config = new mw.Map( bool );
...
... contents of startup/startup.js:
......
```
Change-Id: I97fea20f17c4865aa4740482f3054532038531f0
Only needed internally by the startup module, which can export it
through private means instead.
Follows 0a8e37f042 and 30ddfc8a77.
Change-Id: I5e54bd51d9b47146e1ef9a64b14c1a00697bd6a9
This change makes ResourceLoaderStorageVersion a private variable.
The JavaScript global variable wgResourceLoaderStorageVersion is now
removed.
This change makes the JavaScript code smaller.
Change-Id: I8e31b95d4c44ba653bedb6be500011a39bc6abd8
