I removed the duplicate information from the description but should clarify the following:

Right now, the extension has to be disabled to remove a survey and the associated performance penalty. We also ship additional JS unnecessarily to wikis which have QuickSurveys enabled because of T159738.

The extension doesn't need to be disabled to remove a survey. Surveys can be marked as disabled and their configuration won't be sent to the client. However, if there are no enabled surveys, then the ext.quicksurveys.init RL module is still added to the output. Changing this behaviour is covered in the second acceptance criterion.

T159738: Limit default styles loaded by OOUI (65kb after gzipping penalty to run a QuickSurveys) by droping use of oojs-ui-widgets only occurs when the user sees a survey. That being said, we should still fix it.

Thanks for the clarification! So not quite as bad as I first thought :)

When there are no surveys defined, then the ext.quicksurveys.init RL module is not added to the output

So the downside of not adding ext.quicksurveys.init to the page is that enabled surveys will not show instantly (due to cached HTML!) so we should probably retain loading this unconditionally. However this code should be simplified to check the config value and load the rest of the code including ext.quicksurveys.lib conditionally.

pseudo code:

ext.quick.surveys.init would be simplified to:

if ( mw.config.get( 'wgEnabledQuickSurveys' ) ) {
  mw.loader.using( 'ext.quicksurveys.lib' )
}

Thanks for looking at this. I have a question and two recommendations.

Question: Can we quantify and measure the current impact we think this has on users?

Recommendation for proposal as written:

• I support the idea of making ext.quicksurveys.init no longer hard-depend on `ext.quicksurveys.lib. Separating the init from the rest makes sense.
• Today, all three modules (..init, ..lib, ..views) are always loaded together, so they really should've been a single bundle rather than three separate modules. I'd recommend merge the ext.quicksurveys.lib and ext.quicksurveys.views bundles. They're never needed separately and would help towards T202154 goal and script size.
• Update the server-side hook that enqueues the init module to also independently enqueue ext.quicksurveys.lib alongside ext.quicksurveys.init when surveys are enabled. This ensures that in the vast majority of cases, surveys will not have a worse latency than they have today. And it will avoid a significant regression to loadEventEnd that would otherwise happen for those users. I believe that given latency for lazy-load can be upwards from 3 seconds, it would also negatively impact survey impressions and diversity of the answers if we don't preload in the common case. This means, when surveys just got activated, during the first 24 hours some users lazy-load and some preload. And during the 24h after a survey ends, a few still have it preloaded as the cache churns back. This seems quite acceptable in my opinion, given how this is still relatively small impact overall. But, I think we could be more ambitious here and not even pay this cost. See alternate proposal :)

Alternate proposal:

As Jon's comment at T213459#4870585 shows, really even the remaining init logic could be moved to lib. The only thing we need to is to check the config. And as shown above, this could be checked server-side as well. And if you do both, the client check will be a no-op most cases (rides along the existing promise). That's fine, but we can do better.

The startup module doesn't have to be static. It provides a lot of power, and a lot of the major wins came from embracing that power. This is what we ended up doing with CentralNotice in 2015, worked out really well, and was actually quite easy in retrospect.

It works like this:

• Merge the two view and lib modules (as before).
• Enqueue ..init on all pages unconditionally (as before).
• Vary the definition of the init module in a FileModule subclass by site config. This means, the module is defined as empty and dependency-free when there are no surveys; And empty with lib as its dependency (basically, an alias) when there are surveys.

Outcome: When surveys are enabled, all pages (including cached) instantly start loading lib without lazy-load or delay. When surveys are disabled, all pages (including cached) stop loading lib, and not even the three lines of code for init.

Implementation sketch:

{
 "modules": {
    "ext.quicksurveys.lib": ..,
    "ext.quicksurveys.init": { "class": "MediaWiki\\QuickSurveys\\Module" }
  }
}

class Module extends \ResourceLoaderFileModule {
  getDependencies( $context ) {
    return $context->getConfig()->get('EnabledSurveys') ? [ 'ext.quicksurveys.lib' ] : [];
  }
}

@Krinkle your suggestions sound great to me.

Very neat, @Krinkle! That approach'll require a fair bit of explanation in the QuickSurveys documentation for future developers (covered by AC 3)

Change 561894 had a related patch set uploaded (by Phuedx; owner: Phuedx):
[mediawiki/extensions/QuickSurveys@master] [WIP] [init] Implement zero-cost conditional load of lib

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

Change 561895 had a related patch set uploaded (by Phuedx; owner: Phuedx):
[mediawiki/extensions/QuickSurveys@master] [init] Further reduce cost of conditional loading

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

Change 561894 merged by jenkins-bot:
[mediawiki/extensions/QuickSurveys@master] Implement zero-cost conditional load of lib

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

Change 561895 merged by jenkins-bot:
[mediawiki/extensions/QuickSurveys@master] Further reduce unnecessary conditional loading

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

In T213459#5802581, @gerritbot wrote:

Change 561895 merged by jenkins-bot:
[mediawiki/extensions/QuickSurveys@master] Further reduce unnecessary conditional loading

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

The optimisation added in this commit is conditional loading (using RL skipFunction) based on DNT being enabled. I'd recommend against this for two reasons:

1. The vast majority of users are now paying an additional penalty in the form of downloading the serialised skipFunction on all page views as part of the startup module, which is high turnover module, re-downloads frequently, andexecutes before any regular module can be discovered/requested (blocking). The added bits here aren't large, but I have to also consider the precedent it would set for other logging-related modules, which could slowlo form a more significant regression.

2. DNT itself is an out-going technology. The original premise behind it did not pan out as expected. The working group behind it was disbandoned with the draft standard having been demoted from Candidate Recommendation to Working Group Note. Apple has removed the DNT option from Safari, and other browsers are expected to follow. In any event, very few websites supported it during the experimental phase, and there is no encouragement to promote it further to either website authors nor browser users. (https://en.wikipedia.org/wiki/Do_Not_Track, https://github.com/w3c/dnt/commits, https://www.w3.org/TR/tracking-dnt/#sotd). See also last week's inline comment.

If I understand correctly, for most users (not DNT), the module already has near-zero cost when there are no campaigns enabled (as it would have no source code and no dependencies, and the empty module gets shipped as part of a large batch with unrelated modules, as small as can be). And when there are campaigns enabled, this optimisation does not make it smaller for those users.

The regression seems small enough to not block the train for, so no immediate rush. And I may've misunderstood the change, so I'll keep this as-is and check back over the next 2-3 weeks. Ideally resolving it by then (either by deciding to keep or to revert).

Change 565017 had a related patch set uploaded (by Phuedx; owner: Phuedx):
[mediawiki/extensions/QuickSurveys@master] Partially revert I1aecbf77

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

Change 565018 had a related patch set uploaded (by Phuedx; owner: Phuedx):
[mediawiki/extensions/QuickSurveys@master] Don't show surveys on the main page

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

Change 565017 merged by jenkins-bot:
[mediawiki/extensions/QuickSurveys@master] Partially revert I1aecbf77

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

Change 565018 merged by jenkins-bot:
[mediawiki/extensions/QuickSurveys@master] Don't show surveys on the main page

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

Change 568974 had a related patch set uploaded (by Phuedx; owner: Phuedx):
[mediawiki/extensions/QuickSurveys@master] Revert "Don't show surveys on the main page"

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

Change 568974 merged by jenkins-bot:
[mediawiki/extensions/QuickSurveys@master] Revert "Don't show surveys on the main page"

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