Last modified: 2014-09-03 14:57:06 UTC
Flow adds some modules with both addModuleStyles() and addModules(), e.g. addModuleStyles( array( 'ext.flow.header' ) ); addModules( array( 'ext.flow.header' ) ); The first adds the CSS in a <link rel="stylesheet">, the second adds the same CSS in a JavaScript mw.loader.load() call that appends to a <style> tag in the document. It's a performance hit to add CSS twice and the duplicated Flow CSS rules in browsers' CSS inspector make debugging harder. addModuleStyles() should only be used to load a separate 'flow.<somefeature>.styles' module that only has the LESS and CSS files for the no-JavaScript version. The LESS files for this could be named <something>-nojs.less. addModules( 'flow.<somefeature>' ) should only include LESS and CSS if it's only referenced by the module's JavaScript-generated HTML, such as a JavaScript dialog with unique CSS classes. ^ BTW performance: you don't need to pass an array if it's just one module.
Ori Livneh also pointed out in e-mail that Flow issues multiple separate web requests for RL modules, see http://www.webpagetest.org/result/140220_6G_C4S/1/details/ That should also be addressed in any module refactoring.
Request 4: https://bits.wikimedia.org/en.wikipedia.org/load.php?debug=false&lang=en&modules=ext.flow.base%2Cdiscussion%2Cheader%2Cmoderation&only=styles&skin=vector&* Request 15: https://bits.wikimedia.org/en.wikipedia.org/load.php?debug=false&lang=en&modules=ext.flow.base%2Cdiscussion%2Ceditor%2Cparsoid%7Cjquery.scroll&skin=vector&version=20140220T025329Z&* Request 17: https://bits.wikimedia.org/en.wikipedia.org/load.php?debug=false&lang=en&modules=ext.flow.editors.none&skin=vector&version=20140213T190719Z&* Request 19: https://bits.wikimedia.org/en.wikipedia.org/load.php?debug=false&lang=en&modules=ext.flow.header&skin=vector&version=20140220T025329Z&* That is: Request 1 (<link>; load.php?only=styles; short unversioned cache): ext.flow.base (styles) ext.flow.discussion (styles) ext.flow.header (styles) Request 2 (mw.loader; long versioned cache): ext.flow.base (scripts, styles, messages) ext.flow.discussion (scripts, styles, messages) ext.flow.editor (scripts, styles, messages) ext.flow.parsoid (scripts, styles, messages) Request 3 (mw.loader; long versioned cache): ext.flow.editors.none (scripts, styles, messages) Request 4 (mw.loader; long versioned cache): ext.flow.header (scripts, styles, messages) This loads base, discussion, header styles twice. And has 2 redundant http requests. It should be possible to merge request 3 and 4 into request 2, and change the modules to not contain styles loaded by addModuleStyles in the same module as one loaded with addModules. When working on this be careful with caching. Unless Flow is not deployed anywhere other than test wikis, or only deployed for logged in users, page html is cached for 30 days. Which means load.php?modules=ext.flow.base&only=styles and mw.loader.load('ext.flow.base') should remain working for a while to come (as those are already baked in the html).
As S already pointed out: we need the addModuleStyles() to apply CSS for non-JS users. For both JS & non-JS, we'll probably want to share 95% of the CSS. We should probably assume nojs by default, add all no-JS CSS into a (or multiple) separate module(s) and loadModuleStyles() there. The other modules (with JS) can have some additional CSS for the added JS-functionality then, which is loaded by addModules(). -- Request 3 should be able to be folded into request 2. The reason this is currently split up is that users will some day get to pick their editor. JS detects what editor is active for that user, and loads that specific module (in this caseL editors.none). We should either load all potential editors at once (currently only 1), or defer loading the editor until a user has initiated a reply/edit. -- I'm not really sure why request 4 is seperate. Pretty sure we should be able to merge it into request 2. -- Shahyar is currently working on a fairly big JS refactor
Submitted this too early... As I was saying: Shahyar is currently working on a fairly big JS refactor & I've added him to this ticket just so he's aware. I'm pretty sure there will be some drastic changed to the whole CSS/JS, so it might make more sense to punt this until we're implementing those changes?
Not a problem any more.
