Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

ResourceLoader: Drop targets system, deprecated in 1.41

Browse source 
Bug: T340802
Depends-On: Ie936afed7042d5a4713b027c30d7487565a35eaf
Change-Id: Icad30d62301be5d7390ebdf34e818519e3fe56c4
This commit is contained in:
James D. Forrester 2023-11-23 21:35:21 -05:00
parent 230a8f607a
commit 273cc042ae
18 changed files with 13 additions and 229 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
RELEASE-NOTES-1.42
View file

@ -129,6 +129,11 @@ because of Phabricator reports.
* …
=== Breaking changes in 1.42 ===
* (T340802) ResourceLoader: The targets system is no longer active; all modules
will load on all platforms even if they are marked as desktop- or mobile-only
in the 'targets' field, which is no longer read. As part of this, the internal
methods RL\Module::setTarget() & ::getTargets(), and OutputPage::setTarget(),
have been removed.
* TitleArray, deprecated since 1.41, has been removed.
* UserRightsProxy, deprecated since 1.38, has been removed.
* UserLoginCompleteHook is always called with its $direct parameter set to

12
docs/config-schema.yaml
View file

@ -3484,12 +3484,6 @@ config-schema:
statement. The contents of the file should not contain any wrapping function,
it will be wrapped by %ResourceLoader in an anonymous function and invoked
when the module is considered for loading.
- targets `{string[]}`
List of %ResourceLoader targets where the module may be loaded from.
This is used by the MobileFrontend extension to prevent certain modules
from being loaded.
This option is **deprecated**. See [T127268](https://phabricator.wikimedia.org/T127268).
Default: `[ 'desktop', 'mobile' ]`
## FileModule options
- localBasePath `{string}`:
Base file path to prepend to relative file paths specified in other options.
@ -3610,12 +3604,6 @@ config-schema:
```
return typeof SomeWebAPI === 'function' && SomeWebAPI.prototype.duckMethod;
```
**Example: Module targets**
```
$wgResourceModules['ext.myExtension'] = [
'targets' => [ 'desktop', 'mobile' ],
];
```
@anchor wgResourceModules-example-stylesheet
**Example: Stylesheets**
```

4
docs/extension.schema.v1.json
View file

@ -387,7 +387,7 @@
},
"targets": {
"type": [ "string", "array" ],
"description": "ResourceLoader target the module can run on",
"description": "ResourceLoader target the module can run on (unused since MediaWiki 1.42)",
"items": {
"type": "string"
}
@ -430,7 +430,7 @@
},
"targets": {
"type": [ "string", "array" ],
"description": "ResourceLoader target the module can run on",
"description": "ResourceLoader target the module can run on (unused since MediaWiki 1.42)",
"items": {
"type": "string"
}

6
docs/extension.schema.v2.json
View file

@ -416,7 +416,7 @@
},
"targets": {
"type": [ "string", "array" ],
"description": "ResourceLoader target the module can run on",
"description": "ResourceLoader target the module can run on (unused since MediaWiki 1.42)",
"items": {
"type": "string"
}
@ -459,7 +459,7 @@
},
"targets": {
"type": [ "string", "array" ],
"description": "ResourceLoader target the module can run on",
"description": "ResourceLoader target the module can run on (unused since MediaWiki 1.42)",
"items": {
"type": "string"
}
@ -697,7 +697,7 @@
},
"targets": {
"type": [ "string", "array" ],
"description": "ResourceLoader target the module can run on",
"description": "ResourceLoader target the module can run on (unused since MediaWiki 1.42)",
"items": {
"type": "string"
}

17
includes/MainConfigSchema.php
View file

@ -5568,15 +5568,6 @@ class MainConfigSchema {
* it will be wrapped by %ResourceLoader in an anonymous function and invoked
* when the module is considered for loading.
*
* - targets `{string[]}`
* List of %ResourceLoader targets where the module may be loaded from.
* This is used by the MobileFrontend extension to prevent certain modules
* from being loaded.
*
* This option is **deprecated**. See [T127268](https://phabricator.wikimedia.org/T127268).
*
* Default: `[ 'desktop', 'mobile' ]`
*
* ## FileModule options
*
* - localBasePath `{string}`:
@ -5743,14 +5734,6 @@ class MainConfigSchema {
* ```
* return typeof SomeWebAPI === 'function' && SomeWebAPI.prototype.duckMethod;
* ```
*
* **Example: Module targets**
*
* ```
* $wgResourceModules['ext.myExtension'] = [
* 'targets' => [ 'desktop', 'mobile' ],
* ];
* ```
* @anchor wgResourceModules-example-stylesheet
*
* **Example: Stylesheets**

30
includes/Output/OutputPage.php
View file

@ -627,31 +627,12 @@ class OutputPage extends ContextSource {
if ( $module instanceof RL\Module
&& $module->getOrigin() <= $this->getAllowedModules( $type )
) {
if ( $this->mTarget && !in_array( $this->mTarget, $module->getTargets() ) ) {
$this->warnModuleTargetFilter( $module->getName() );
continue;
}
$filteredModules[] = $val;
}
}
return $filteredModules;
}
private function warnModuleTargetFilter( $moduleName ) {
static $warnings = [];
if ( isset( $warnings[$this->mTarget][$moduleName] ) ) {
return;
}
$warnings[$this->mTarget][$moduleName] = true;
$this->getResourceLoader()->getLogger()->debug(
'Module "{module}" not loadable on target "{target}".',
[
'module' => $moduleName,
'target' => $this->mTarget,
]
);
}
/**
* Get the list of modules to include on this page
*
@ -712,15 +693,6 @@ class OutputPage extends ContextSource {
return $this->mTarget;
}
/**
* Sets ResourceLoader target for load.php links. If null, will be omitted
*
* @param string|null $target
*/
public function setTarget( $target ) {
$this->mTarget = $target;
}
/**
* Force the given Content object for the given page, for things like page preview.
* @see self::addContentOverrideCallback()
@ -3701,7 +3673,7 @@ class OutputPage extends ContextSource {
* @return string|WrappedStringList HTML
*/
public function makeResourceLoaderLink( $modules, $only, array $extraQuery = [] ) {
// Apply 'target' and 'origin' filters
// Apply 'origin' filters
$modules = $this->filterModules( (array)$modules, null, $only );
return RL\ClientHtml::makeLoad(

11
includes/ResourceLoader/FileModule.php
View file

@ -218,7 +218,6 @@ class FileModule extends Module {
// Lists of strings
case 'dependencies':
case 'messages':
case 'targets':
// Normalise
$option = array_values( array_unique( (array)$option ) );
sort( $option );
@ -602,7 +601,6 @@ class FileModule extends Module {
// this affects 'scripts' and other file paths, getFileHashes accounts for that.)
// - remoteBasePath (Per T104950)
// - dependencies (provided via startup module)
// - targets
// - group (provided via startup module)
'styles',
'skinStyles',
@ -1047,15 +1045,6 @@ class FileModule extends Module {
return $context->getDirection() === 'rtl' && !$this->noflip;
}
/**
* Get target(s) for the module, eg ['desktop'] or ['desktop', 'mobile']
*
* @return string[]
*/
public function getTargets() {
return $this->targets;
}
/**
* Get the module's load type.
*

12
includes/ResourceLoader/Module.php
View file

@ -63,8 +63,6 @@ abstract class Module implements LoggerAwareInterface {
/** @var string|null Module name */
protected $name = null;
/** @var string[] What client platforms the module targets (e.g. desktop, mobile) */
protected $targets = [ 'desktop', 'mobile' ];
/** @var string[]|null Skin names */
protected $skins = null;
@ -492,16 +490,6 @@ abstract class Module implements LoggerAwareInterface {
return [];
}
/**
* Get target(s) for the module, eg ['desktop'] or ['desktop', 'mobile']
*
* @stable to override
* @return string[]
*/
public function getTargets() {
return $this->targets;
}
/**
* Get list of skins for which this module must be available to load.
*

19
includes/ResourceLoader/StartUpModule.php
View file

@ -34,10 +34,6 @@ use Wikimedia\RequestTimeout\TimeoutException;
* the ability to vary based extra query parameters, in addition to those
* from Context:
*
* - target: Only register modules in the client intended for this target.
* Default: "desktop".
* See also: OutputPage::setTarget(), Module::getTargets().
*
* - safemode: Only register modules that have ORIGIN_CORE as their origin.
* This disables ORIGIN_USER modules and mw.loader.store. (T185303, T145498)
* See also: OutputPage::disallowUserJs()
@ -163,11 +159,8 @@ class StartUpModule extends Module {
$resourceLoader = $context->getResourceLoader();
// Future developers: Use WebRequest::getRawVal() instead getVal().
// The getVal() method performs slow Language+UTF logic. (f303bb9360)
$target = $context->getRequest()->getRawVal( 'target', 'desktop' );
$safemode = $context->getRequest()->getRawVal( 'safemode' ) === '1';
$skin = $context->getSkin();
// Allow disabling target filter, for use by SpecialJavaScriptTest.
$byPassTargetFilter = $this->getConfig()->get( MainConfigNames::EnableJavaScriptTest ) && $target === 'test';
$moduleNames = $resourceLoader->getModuleNames();
@ -191,19 +184,9 @@ class StartUpModule extends Module {
$registryData = [];
foreach ( $moduleNames as $name ) {
$module = $resourceLoader->getModule( $name );
$moduleTargets = $module->getTargets();
$moduleSkins = $module->getSkins();
$targetMismatch = !in_array( 'mobile', $moduleTargets ) || !in_array( 'desktop', $moduleTargets );
$hasValidTarget = in_array( 'mobile', $moduleTargets ) || in_array( 'desktop', $moduleTargets );
if ( $hasValidTarget && $targetMismatch ) {
wfDeprecated(
'Modules must target desktop and mobile. Module name:' . $name,
'1.41'
);
}
if (
( !$byPassTargetFilter && !in_array( $target, $moduleTargets ) )
|| ( $safemode && $module->getOrigin() > Module::ORIGIN_CORE_INDIVIDUAL )
( $safemode && $module->getOrigin() > Module::ORIGIN_CORE_INDIVIDUAL )
|| ( $moduleSkins !== null && !in_array( $skin, $moduleSkins ) )
) {
continue;

1
includes/ResourceLoader/WikiModule.php
View file

@ -111,7 +111,6 @@ class WikiModule extends Module {
case 'scripts':
case 'datas':
case 'group':
case 'targets':
$this->{$member} = $option;
break;
}

1
includes/ServiceWiring.php
View file

@ -1868,7 +1868,6 @@ return [
],
$msgPosterAttrib['dependencies'] ?? []
),
'targets' => [ 'desktop', 'mobile' ],
] );
if ( $config->get( MainConfigNames::EnableJavaScriptTest ) === true ) {

2
includes/registration/ExtensionProcessor.php
View file

@ -647,8 +647,6 @@ class ExtensionProcessor implements Processor {
$data['localBasePath'] = "$dir/{$data['localBasePath']}";
}
}
// Satisfy PHPUnit ResourcesTest::testUnsatisfiableDependencies
$data['targets'] = [ 'test' ];
$this->attributes['QUnitTestModules']["test.{$info['name']}"] = $data;
}

1
tests/phpunit/ResourceLoaderTestCase.php
View file

@ -123,7 +123,6 @@ class ResourceLoaderTestModule extends Module {
protected $isRaw = false;
protected $isKnownEmpty = false;
protected $type = Module::LOAD_GENERAL;
protected $targets = [ 'phpunit' ];
protected $shouldEmbed = null;
protected $mayValidateScript = false;

10
tests/phpunit/includes/Output/OutputPageTest.php
View file

@ -502,16 +502,6 @@ class OutputPageTest extends MediaWikiIntegrationTestCase {
);
}
// @todo How to test filterModules(), warnModuleTargetFilter(), getModules(), etc.?
public function testSetTarget() {
$op = $this->newInstance();
$op->setTarget( 'foo' );
$this->assertSame( 'foo', $op->getTarget() );
// @todo What else? Test some actual effect?
}
// @todo How to test addContentOverride(Callback)?
public function testHeadItems() {

29
tests/phpunit/includes/ResourceLoader/StartUpModuleTest.php
View file

@ -277,26 +277,6 @@ mw.loader.register([
[],
3
]
]);'
] ],
[ [
'msg' => 'Different target (non-test should not be registered)',
'modules' => [
'test.blank' => [ 'class' => ResourceLoaderTestModule::class ],
'test.target.foo' => [
'class' => ResourceLoaderTestModule::class,
'targets' => [ 'x-foo' ],
],
],
'out' => '
mw.loader.addSource({
"local": "/w/load.php"
});
mw.loader.register([
[
"test.blank",
""
]
]);'
] ],
[ [
@ -597,15 +577,6 @@ mw.loader.register([
'group' => 'x-bar',
'source' => 'example',
],
'test.target.foo' => [
'class' => ResourceLoaderTestModule::class,
'targets' => [ 'x-foo' ],
],
'test.target.bar' => [
'class' => ResourceLoaderTestModule::class,
'source' => 'example',
'targets' => [ 'x-foo' ],
],
'test.es6' => [
'class' => ResourceLoaderTestModule::class,
]

79
tests/phpunit/structure/ResourcesTest.php
View file

@ -38,7 +38,7 @@ class ResourcesTest extends MediaWikiIntegrationTestCase {
* exist and are not illegal.
*
* @todo Modules can dynamically choose dependencies based on context. This method
* does not find all such variations. The same applies to testUnsatisfiableDependencies().
* does not find all such variations.
*/
public function testValidDependencies() {
$data = self::getAllModules();
@ -98,83 +98,6 @@ class ResourcesTest extends MediaWikiIntegrationTestCase {
}
}
/**
* Verify that dependencies of all modules are actually registered in the same client context.
*
* Example:
* - A depends on B. A has targets: mobile, desktop. B has targets: desktop. Therefore the
* dependency is sometimes unregistered: it's impossible to load module A on mobile.
*/
public function testUnsatisfiableDependencies() {
$data = self::getAllModules();
/** @var RL\Module $module */
$incompatibleTargetNames = [];
$targetsErrMsg = '';
foreach ( $data['modules'] as $moduleName => $module ) {
$depNames = $module->getDependencies( $data['context'] );
$moduleTargets = $module->getTargets();
foreach ( $depNames as $depName ) {
$dep = $data['modules'][$depName] ?? null;
if ( !$dep ) {
// Missing dependencies reported by testMissingDependencies
continue;
}
if ( $moduleTargets === [ 'test' ] ) {
// Target filter does not apply under tests, which may include
// both mobile-only and desktop-only dependencies.
continue;
}
$targets = $dep->getTargets();
foreach ( $moduleTargets as $moduleTarget ) {
if ( !in_array( $moduleTarget, $targets ) ) {
$incompatibleTargetNames[] = $moduleName;
$targetsErrMsg .= "* The module '$moduleName' must not have target '$moduleTarget' "
. "because its dependency '$depName' does not have it\n";
}
}
}
}
$this->assertEquals( [], $incompatibleTargetNames, $targetsErrMsg );
}
/**
* Verify that dependencies of all modules are actually registered in the same client context.
*
* Example:
* - A depends on B. A has targets: mobile, desktop. B has targets: desktop. Therefore the
* dependency is sometimes unregistered: it's impossible to load module A on mobile.
* - A depends on B. B has requiresES6=true but A does not. In some browsers, B will be
* unregistered at startup and thus impossible to satisfy as dependency.
*/
public function testRedundantTargets() {
$targetsBad = [];
$data = self::getAllModules();
// This makes sure that new modules are not added in a way that goes against
// the current plan to dismantle the targets system.
// Modules should only be removed from the list, not added.
$knownExceptions = [];
foreach ( $data['modules'] as $moduleName => $module ) {
$definedTargets = $module->getTargets();
if (
!in_array( $moduleName, $knownExceptions ) &&
!str_starts_with( $moduleName, 'test.' ) &&
(
!in_array( 'desktop', $definedTargets ) ||
!in_array( 'mobile', $definedTargets )
)
) {
$targetsBad[] = $moduleName;
}
}
$this->assertEquals( [], $targetsBad,
'All modules should load on both mobile and desktop target. '
. 'The following modules have redundant targets definitions:' . implode( ' ', $targetsBad )
);
}
/**
* Get all registered modules from ResouceLoader.
* @return array

1
tests/phpunit/unit/includes/registration/ExtensionProcessorTest.php
View file

@ -945,7 +945,6 @@ class ExtensionProcessorTest extends MediaWikiUnitTestCase {
'localBasePath' => $dir,
'remoteExtPath' => 'Foo',
'scripts' => 'bar.js',
'targets' => [ 'test' ],
],
],
],

2
tests/qunit/QUnitTestResources.php
View file

@ -9,7 +9,6 @@ return [
'tests/qunit/data/sinonjs-local.js',
'resources/lib/sinonjs/sinon.js',
],
'targets' => [ 'desktop', 'mobile' ],
],
'mediawiki.qunit-testrunner' => [
@ -20,7 +19,6 @@ return [
'mediawiki.page.ready',
'sinonjs',
],
'targets' => [ 'desktop', 'mobile' ],
],
'mediawiki.language.testdata' => [