Quick notes on the migration path:

*Cached page HTML*

HTML cached under the old regime would still be served for a while, but
ResourceLoader would load the newer style. I *think* that should work as
long as the new versions of the modules that were included before still
list the style-only modules as dependencies, assuming load.php doesn't
throw an exception when the code-plus-style modules are requested.

If load.php would throw an exception in this case, then that's going to be
a tricky problem to figure out for end-users, who will be presented with
unstyled pages and no visible error message.


*Extension authors and users of extensions*

If addModuleStyles() throws a PHP exception at page-output time for modules
that include scripts, that's a breaking change for extension authors and
the people who use their extensions; but at least the error message will be
very direct and immediate.

It'll need to be called out in the release notes with a section about
breaking upgrade changes.

Ideally, the resulting error message that is displayed should also be very
clear and include a link to a documentation page explaining how extension
authors can update their code appropriately.

-- brion



On Wed, May 4, 2016 at 3:34 AM, Krinkle <krinklem...@gmail.com> wrote:

> 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).
> _______________________________________________
> Wikitech-l mailing list
> Wikitech-l@lists.wikimedia.org
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
_______________________________________________
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Reply via email to