Page MenuHomePhabricator

Deprecate and remove support for 'object' key in wgResourceModules
Closed, ResolvedPublic

Description

Was reminded of this when reading T222409.

Afaik we don't use this anywhere. I recall it being used for testing at some point, but doesn't seem useful for that either. Can be trivially changed to either mocking the registry, or using factory. (Note: The factory function feature didn't exist when we first released the object key ability.)

The use of object discourages best practices for performance and dependency injection.

This was kept for compatibility with MediaWiki 1.17alpha. Proposing to remove in MW 1.34, so as to reduce the API surface area, reduce maintenance cost, and present a simpler interface for developers that has no fewer capabilities than today.

Event Timeline

Krinkle created this task.May 6 2019, 4:26 PM
Restricted Application added a project: Performance-Team. · View Herald TranscriptMay 6 2019, 4:26 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Krinkle updated the task description. (Show Details)May 6 2019, 6:11 PM
Krinkle updated the task description. (Show Details)
Krinkle triaged this task as Low priority.May 6 2019, 7:52 PM
Krinkle moved this task from Inbox to Accepted Enhancement on the MediaWiki-ResourceLoader board.
Krinkle updated the task description. (Show Details)Jul 11 2019, 5:07 PM

Change 522221 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Remove use of object registering in test suites

https://gerrit.wikimedia.org/r/522221

Change 522222 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Remove support for 'object' in wgResourceModules

https://gerrit.wikimedia.org/r/522222

Change 522221 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Remove use of object registering in test suites

https://gerrit.wikimedia.org/r/522221

Change 522518 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Remove register() 'object' use in OutputPageTest

https://gerrit.wikimedia.org/r/522518

Change 522518 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Remove register() 'object' use in OutputPageTest

https://gerrit.wikimedia.org/r/522518

Change 522222 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Remove support for 'object' in wgResourceModules

https://gerrit.wikimedia.org/r/522222

Yay: https://travis-ci.org/cmln/chameleon/jobs/559626849

Could you provide information on where/when this was deprecated?

Krinkle added a comment.EditedJul 16 2019, 8:31 PM

@Foxtrott It's been a discouraged feature since 2011 really, obsolete with other ways of registering. It was never used in production for MediaWiki core, tarballed, or WMF-deployed projects.

Release notes for 1.34 (not yet released), are at source.

For migrate, you can either: return the object from a factory closure:

// before
register(name, $obj);
// after
register(name, ['factory' => function () use ($obj) {
  return $obj;
});

Or, if the object is a plain construction, I'd recommend the declarative format, like so:

// before
register(name, new MyClass([
  'x' => 'y'
]);
// after
register(name, [
  'class' => MyClass::class,
  'x' => 'y'
]);
Krinkle closed this task as Resolved.Jul 16 2019, 8:32 PM
Krinkle claimed this task.

So, not deprecated and you don't care.

Krinkle added a comment.EditedJul 16 2019, 8:47 PM

It was never intended for use, there were no known uses in any past or present released version of MediaWiki core, WMF production repos, and no current uses anywhere in repositories hosted on Gerrit, and no usage anywhere as found through Codesearch for third-party hosted repositories on GitHub etc.. Per these search queries I did:

With no known usage, and only a small chance of some other patterns I couldn't match or in private/other repos, I decided it would not be worth the overhead and maintenance cost of 6 more months of WMF-funded development resources. And thus to remove without deprecation. This is in line with our Deprecation policy – (I will e-mail Wikitech-l this week as well. I should have done this last week.)