Page MenuHomePhabricator

Improve GuidedTour performance
Open, Needs TriagePublic

Description

Its code base not kept up with current best practices. I consider it a prerequisite for any new use/deployment of the extension, for it to first have an active steward, be updated to current best practices, and specifically to have a significantly better startup performance footprint for regular page views.

Its currently structured in a way that requires each tour to be registered as its own top-level module bundle, which does not scale (incompatible with current guidelines).

Some ideas:

  • one way would be to have the tour code go into one big module, like messageposter. you'd have an extension registry attribute like "GuidedTourModule", and then the guidedtour extension takes those, merges them, and registers them as one file module. This would mean 'scripts', no packageFile module since those cannot easily be merged as simple PHP arrays. see MessagePosterModule for an example of that
  • Another way would be to expand the configuration to take a module name and property name. This would be much simpler, but only solves the problem if the extension isn't perf-sensitive and has another module you can throw the code into. E.g. for CheckUser, you could throw the code in a random module, and then tell guided tour to get it from require('ext.checkUser.stuff').tourA with some JSON declaration
  • Similar to #2, we could invert the way GT works and make individual extensions responsible for loading the tours instead of making GT do it. E.g. centralnotice special page or hooks would queue what they need, and their own code would load and/or construct whatever they need to do, like you would for any other JS on the page.
  • Solve T225842 first, and make these private modules, automatically merged by RL into one "ext.guidedTours.tours" bundle

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

one way would be to have the tour code go into one big module, like messageposter. you'd have an extension registry attribute like "GuidedTourModule", and then the guidedtour extension takes those, merges them, and registers them as one file module. This would mean 'scripts', no packageFile module since those cannot easily be merged as simple PHP arrays. see MessagePosterModule for an example of that

Wouldn't this mean that, any time you wanted to launch a single tour, the javascript for all registered tours would need to be sent to the client, since it would also be a part of the same module? This isn't as big of a deal for message poster, since its just the core version and flow's, that are currently available, but, if we had a single module for all deployed tours, it would include

  • from the GuidedTour extension: firstedit, test, onshow, uprightdownleft
  • from GrowthExperiments: helppanel, homepage_mentor, homepage_welcome, homepage_discovery
  • from GettingStarted: gettingstartedtasktoolbar, gettingstartedtasktoolbarve
  • from CheckUser: checkuserinvestigateform, checkuserinvestigate
  • from Flow: flowOptIn
  • from WikimediaMessages: RcFiltersIntro, WlFiltersIntro, RcFiltersHighlight

one way would be to have the tour code go into one big module, like messageposter. you'd have an extension registry attribute like "GuidedTourModule", and then the guidedtour extension takes those, merges them, and registers them as one file module. This would mean 'scripts', no packageFile module since those cannot easily be merged as simple PHP arrays. see MessagePosterModule for an example of that

Wouldn't this mean that, any time you wanted to launch a single tour, the javascript for all registered tours would need to be sent to the client, since it would also be a part of the same module? This isn't as big of a deal for message poster, since its just the core version and flow's, that are currently available, but, if we had a single module for all deployed tours, it would include

  • from the GuidedTour extension: firstedit, test, onshow, uprightdownleft
  • from GrowthExperiments: helppanel, homepage_mentor, homepage_welcome, homepage_discovery
  • from GettingStarted: gettingstartedtasktoolbar, gettingstartedtasktoolbarve
  • from CheckUser: checkuserinvestigateform, checkuserinvestigate
  • from Flow: flowOptIn
  • from WikimediaMessages: RcFiltersIntro, WlFiltersIntro, RcFiltersHighlight

It depends on their complexity and size. They don't have go into a single bundle per-se. The QUnitTestModule, for example, also follows the above principle, but results in one combined bundle per extension. It's an implementation detail.

I also note that it might actually be fine to have all these in one bundle. From what I recall, guided tour definitions generally aren't particularly complex. It would be a small module even with all the above combined, if we don't include their dependencies. Those can be lazy-loaded (and the tours are lazy-loaded anyway, so this wouldn't make much difference), by declaring those inside the file using mw.loader.using() thus only distributing the code in one bundle.

Another approach I mentioned before that might be worth exploring is to bundle tours with the feature they are for. E.g. Flow tour can be part of an existing Flow module. It doesn't need to be connected to GuidedTours at all (central or not).

The main work that needs to happen for this task is to decouple how tours are registered on the server-side. Currently the design of this registration pattern mandates the existence of a separate module bundle. Once that is decoupled, we can go in many different directions, possibly multiple based on locally-made decisions for that is optimal for those teams and features.