ResourceLoader addModuleStyles() issues - Wikitech-l

4 May 2016

TL;DR: The current addModuleStyles() method no longer meets our
requirements. This mismatch causes bugs (e.g. user styles load twice). The
current logic also doesn't support dynamic modules depending on style
modules. I'm looking for feedback on how to best address these issues.

ResourceLoader is designed with two module types: Page style modules and
dynamic modules.

Page style modules are part of the critical rendering path and should work
without JavaScript being enabled or supported. Their URL must be referenced
directly in the HTML to allow browsers to discover them statically for
optimal performance.  As such, this URL can't use dynamic information from
the startup module (no dependencies[1], no version hashes).

Dynamic modules have their names listed in the page HTML. Their
dependencies and version hashes are resolved at run-time by JavaScript via
the startup manifest. We then generate the urls and request them from the
server. There is also a layer of object caching in-between (localStorage)
which often optimises the module request to not involve an HTTP request at
all.

Below I explain the two issues, followed by a proposed solution.

## Dependency

Historically there was no overlap between these two kinds of modules. Page
style modules were mostly dominated by the skin and apply to skin-specific
server-generated html. Dynamic modules (skin agnostic) would make their own
DOM and apply their own styles.

Now that we're reusing styles more, we're starting seeing to see overlap.
In the past we used jQuery UI - its styles never applied to PHP-generated
output. Now, with OOUI, its styles do apply to both server-side and
client-side generated elements. Its styles are preloaded as a page style
module on pages that contain OOUI widgets.

The OOjs UI bundle (and its additional styles) shouldn't need to know
whether the current page already got some of the relevant styles. This is
what dependencies are for. For OOUI specifically, we currently have a
hardcoded work around that adds a hidden marker to pages where OOUI is
preloaded. The OOUI style module has a skipFunction that forgoes loading if
the marker is present.

## Implicit type

Aside from the need for dependencies between dynamic and page style
modules. There is another bug related to this. We don't currently require
modules to say whether they are a dynamic module or a page style module. In
most cases this doesn't matter since a developer would only load it one way
and the module type is implied. For example, if you only ever pass a module
to addModules() or mw.loader() it is effectively a dynamic module. If you
only ever pass a module to addModuleStyles() loaded via a <link
rel=stylesheet> then it is a page style module.

A problem happens if one tries to load the same module both ways. This
might seem odd (and it is), but happens unintentionally right now with wiki
modules (specifically, user/site modules and gadgets).

For user/site modules, we don't know whether common.css relates to the page
content, or whether it relates to dynamic content produced by common.js.
The same for gadgets. A gadget may produce an AJAX interface and register
styles for it, or the gadget may be styles-only and intended as a skin
customisation. Right now the Gadgets extension works around this problem
with a compromise: Load it both ways. First it loads all gadgets as page
style modules (ignoring the script portion), and then it loads the same
modules again as dynamic modules. Thus loading the styles twice.

## Proposed solution

In order to allow dependency relationships between a dynamic module and a
page style module, we need to inform mw.loader in JavaScript about which
modules have already been loaded by different means. We do this already
with the user modules (by setting mw.loader.state directly).

This would work but means that if you then load the same module again as
dynamic module, it will assume the module is already loaded and thus never
deliver the script portion (for the case where the gadget wasn't meant to
be a page style module). Similarly, it would mean that common.js wouldn't
get delivered if common.css exists.

For user/site modules I propose to solve this by splitting them up into
'user', 'user.styles', 'site' and 'site.styles'. Existing
dependency
relationships between other modules and 'user' or 'site' will continue to
work. It'd mostly be an internal detail. This allows us to load one as a
page style module and the other dynamically.

For gadgets we can try to infer the intent (styles-only = page style
module, both = dynamic module), with perhaps a way to declare the desired
load method explicitly in Gadgets-definition if the default is wrong.

With that resolved, we can export mw.loader.state information for page
style modules, which then allows dynamic modules to depend on them.

Thoughts?

https://phabricator.wikimedia.org/T87871
https://phabricator.wikimedia.org/T92459

-- Krinkle

[1] If one would allow page style modules to have dependencies and resolve
them server-side in the HTML output, this would cause corruption when the
relationship between two modules changes as existing pages would have the
old relationship cached but do get the latest content from the server.
Adding versions wouldn't help since the server can't feasibly have access
to previous versions (too many page/skin/language combinations).