Page MenuHomePhabricator

MobileFrontend modules: Cleanup some deprecation calls
Closed, ResolvedPublic5 Story Points

Description

While we've been migrating code to webpack, for backwards compatibility we've been making use of the deprecate module function and changing function signatures. These lead to console deprecation warnings.

This task will be updated as more modules make use of it, and we'll squash those in one clean swoop!

Acceptance criteria

  • ag "mobileFrontend.deprecate" src/ shows zero results
  • All notices reporting "Setting events on the View is deprecated. Please use options." are fixed.
  • DEPRECATED_PROPERTIES inside src/mobile.startup/View.js is an empty array. (was removed entirely)
  • Any modules defined using M.define that are not used by Minerva are removed.
  • ag "this.events" in MobileFrontend matches nothing
  • Ensure matches for ag M.define match M.require calls in Minerva - e.g. we're not porting anything we don't need to be.

Developer notes

Fixing this.events

The following files need fixes:

  • resources/skins.minerva.options/BackToTopOverlay.js (move to props, drop redundant View.prototype.events)
  • resources/skins.minerva.scripts/init.js (update initRedLinks to drop CtaDrawer.prototype.events)
  • src/mobile.mediaViewer/LoadErrorMessage.js (move events to View.call)
  • src/mobile.startup/Overlay.js (move events to View.call... caution - make sure all Overlay's are not passing events)
  • src/mobile.startup/Panel.js
  • src/mobile.startup/references/ReferencesDrawer.js (move events to View.call)
  • src/mobile.startup/watchstar/Watchstar.js (move events to View.call)
  • src/mobile.startup/Skin.js (empty events object - can simply be removed)

Fixing the modules

https://codesearch.wmflabs.org/search/?q=M.require&i=nope&files=&repos=
Will let you know which modules need to be exposed to other extensions. Don't expose anything that doesn't need to be!

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 7 2018, 12:14 AM
Jdlrobson renamed this task from Cleanup some deprecation calls to MobileFrontend modules: Cleanup some deprecation calls.Nov 7 2018, 12:14 AM
phuedx triaged this task as Normal priority.Nov 8 2018, 11:49 AM
phuedx added a subscriber: phuedx.

Is it fair to say that this work is blocked on the rest of the refactoring work (or, at least, should be scheduled at the end of it?).

Jdlrobson changed the task status from Open to Stalled.Nov 8 2018, 5:44 PM

yep. adding a parent task and marking as stalled.

Niedzielski updated the task description. (Show Details)Jan 9 2019, 8:40 PM
Niedzielski added a subscriber: Niedzielski.
Jdlrobson changed the task status from Stalled to Open.Jan 18 2019, 7:25 PM
Jdlrobson raised the priority of this task from Normal to High.
Jdlrobson updated the task description. (Show Details)
Jdlrobson set the point value for this task to 5.Jan 22 2019, 5:49 PM

Change 488127 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Update: add View.events support to Panels

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

Change 488128 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/skins/MinervaNeue@master] Hygiene: migrate View.events to constructor prop

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

Change 488127 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Update: add View.events support to Panels

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

Change 488128 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Hygiene: migrate View.events to constructor prop

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

Change 488589 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Breaking: replace View.events with constructor args

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

Change 488962 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/skins/MinervaNeue@master] Hygiene: replace mobile.startup/paths with props

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

Change 488964 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/ZeroBanner@master] Remove ZeroBanner interstitial

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

Change 488969 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Breaking: remove mobile.editor ResourceLoader module

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

Change 488971 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Hygiene: remove unused Deferred deprecation warnings

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

Change 488983 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Hygiene: migrate Deferred.fail() use and remove warning

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

Change 488987 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Hygiene: drop support for deprecated ResourceLoader modules

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

Change 488589 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Breaking: replace View.events with constructor args

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

Change 488962 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Hygiene: replace mobile.startup/paths with props

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

Change 488969 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Breaking: remove mobile.editor ResourceLoader module

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

Change 488971 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Hygiene: remove unused Deferred deprecation warnings

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

Change 488983 merged by Jdlrobson:
[mediawiki/extensions/MobileFrontend@master] Hygiene: migrate Deferred.fail() use and remove warning

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

Change 489254 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Breaking: replace mobile.startup/* ResourceLoader definitions

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

Change 489255 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Breaking: trim unused mobile.startup exports

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

Change 489257 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/skins/MinervaNeue@master] Hygiene: rename mobile.startup variable in search

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

Change 489259 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/skins/MinervaNeue@master] Hygiene: replace OO.mfExtend with mobile.mfExtend

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

Change 489261 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Breaking: replace OO.mfExtend with mobile.mfExtend

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

Change 489254 abandoned by Jdlrobson:
Breaking: replace mobile.startup/* ResourceLoader definitions

Reason:
Squashed into https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/MobileFrontend/ /488987/4

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

Change 488987 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Breaking: replace mobile.startup/* ResourceLoader definitions

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

Change 489255 merged by Jdlrobson:
[mediawiki/extensions/MobileFrontend@master] Breaking: trim unused mobile.startup exports

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

Change 489257 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Hygiene: rename mobile.startup variable in search

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

Change 489259 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Hygiene: replace OO.mfExtend with mobile.mfExtend

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

Change 489261 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Breaking: replace OO.mfExtend with mobile.mfExtend

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

Change 489801 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/skins/MinervaNeue@master] Hygiene: update categoryOverlay to use factory API

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

Change 489802 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Breaking: remove deprecated CategoryOverlay API

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

Change 489888 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/skins/MinervaNeue@master] Hygiene: update talkOverlay to use factory API

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

Change 489890 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Breaking: remove deprecated TalkOverlay API

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

Change 489895 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/MobileFrontend@master] Doc: revise mobile.editor.overlay comment

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

Change 489888 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Hygiene: update talkOverlay to use factory API

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

Change 489801 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Hygiene: update categoryOverlay to use factory API

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

Change 489802 merged by Jdlrobson:
[mediawiki/extensions/MobileFrontend@master] Breaking: remove deprecated CategoryOverlay API

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

Change 489890 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Breaking: remove deprecated TalkOverlay API

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

Change 489895 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Doc: revise mobile.editor.overlay comment

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

Change 488964 merged by jenkins-bot:
[mediawiki/extensions/ZeroBanner@master] Remove ZeroBanner interstitial

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

nray claimed this task.Feb 13 2019, 6:05 PM
nray closed this task as Resolved.Feb 13 2019, 8:59 PM
nray removed nray as the assignee of this task.
nray updated the task description. (Show Details)
nray added a subscriber: nray.

It looks like I621b922bbf30bd3d31dc83b1fe06dc31e9935dd1 led to the error spike, we're seeing here:

I replicated this on a page which recently throw a client error:

We should increase our deprecation time to 2 weeks in future.

We should increase our deprecation time to 2 weeks in future.

@Jdlrobson, is this for all API breaking changes or only those removing modules from extension.json?

We should increase our deprecation time to 2 weeks in future.

@Jdlrobson, is this for all API breaking changes or only those removing modules from extension.json?

This would be for any breaking changes in HTML. extension.json removals are only a problem if OutputPage->addModule or OutputPage->addModuleStyles is called with the module name as these add HTML references to the modules. Usually we assume a cache time of 1 week but it might be better to bump that to 2 weeks from now on.