Background:
- In T262167: Add a guided tour for settings I added a GuidedTour tour to Special:GlobalWatchlistSettings to be shown if the GuidedTour extension was installed, and a user had no settings saved (which usually indicates that they haven't tried the functionality before).
- In T264833: Add an option to disable GuidedTour for Special:GlobalWatchlistSettings we added a configuration setting, $wgGlobalWatchlistEnableGuidedTour, to control if the tour was available (disabled by default) and would not register it unless that setting was true, due to performance concerns about the additional ResourceLoader module that was being registered. (GuidedTour generally requires a dedicated module for each tour, see T264817: Improve GuidedTour performance for more).
Status quo for ResourceLoader modules:
We currently register four different modules:
- ext.globalwatchlist.specialglobalwatchlist for the OOUI/jQuery version of Special:GlobalWatchlist
- ext.globalwatchlist.specialglobalwatchlist.vue for the Vue version of Special:GlobalWatchlist
- ext.globalwatchlist.specialglobalwatchlistsettings a styles-only module for Special:GlobalWatchlistSettings
- ext.globalwatchlist.getsettingserror a tiny module to complain about a user having invalid settings, loaded by either the php code for Special:GlobalWatchlistSettings if the php can't parse the existing user options, or loaded by the JavaScript code for Special:GlobalWatchlist if the JavaScript can't parse the options.
We then also sometimes register ext.guidedTour.globalWatchlistSettings if the GuidedTour extension is installed and $wgGlobalWatchlistEnableGuidedTour is enabled.
The only JavaScript that is loaded on Special:GlobalWatchlistSettings is in ext.globalwatchlist.getsettingserror, which is currently just a single file, getSettings.error.js. We need a separate ResourceLoader module for the settings page to show this error. However, that doesn't mean that it can't do other things too:
Proposal:
Replace ext.globalwatchlist.getsettingserror with ext.globalwatchlist.settingsErrorOrTour that will, when loaded, either
- invoke the code in getSettings.error.js, or
- require the ext.guidedTour module, and then run the tour for the settings page.
This new module will not have ext.guidedTour as a dependency in its module declaration, but instead only load that at run time if it is told to try and show the tour. This allows us to avoid needing to change the module declaration depending on if the extension is available and the configuration calls for making the tour available.
The javascript will detect whether to show a warning about the settings or to run the tour based on a javascript variable exported from the special page via OutputPage::addJsConfigVars().
Instead of the javascript for Special:GlobalWatchlist loading the error module, they will execute getSettings.error.js directly (it'll be updated to not show the error warning immediately when loaded).
Estimated performance impact:
- We eliminate ever needing to register the tour as a separate ResourceLoader module when its enabled
- We eliminate the need to have a hook handler for the ResourceLoaderRegisterModules hook, which I believe is called on every page view
- We unfortunately need to load the messages for the tour even when they aren't being used, though I expect the impact of that to be negligible (we could instead load them via mw.Api.loadMessages() only when needed, like the ext.guidedTour module, but I expect that would be slower)
- If we try to show the tour the ext.guidedTour dependency is loaded at run time via mw.loader.using which may be slightly slower (though we could get around that by preloading it via OutputPage::addModules for ext.guidedTour in the php code)
Given that we originally disabled the tour because it was registering a separate ResourceLoader module, which it would not under this proposal, I assume this would be okay with the Performance-Team, but I'd like to make sure before starting to work on it.
If this is approved, I'll spend some time improving the tour before we enable it for WMF wikis, so this task isn't about actually enabling it now, just approval to enable it later.