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