Page MenuHomePhabricator

Investigate coupling of ResourceLoader modules used in both places
Closed, ResolvedPublic

Description

  • Hint: Talk to whoever was on module terminators hike for more hints

(Related tickets from @Addshore, T236206: Codes in wikibase/lib/resources/lib are duplicate and untested T237727: Symlinked js files broken on Windows)

Event Timeline

Addshore updated the task description. (Show Details)Jun 11 2020, 9:43 AM

As part of my RL modules terminators, I have some ideas on how the whole thing works.

Restricted Application added a project: User-Ladsgroup. · View Herald TranscriptJun 11 2020, 1:51 PM

Okay, here's my report:
This is the coupling between client RL modules and lib:


As you see only linkitem jquery ui widget has coupling to basically all of lib (it's because I dried lib RL to view for the ones that only was depended by repo)

Also it's worth noting vue2 RL module is being loaded on every wiki but it's not being depended by client at all (it depends on the core's vue module) and it's only needed by WikibaseLexeme. This needs fixing.

When you add repo to the picture, this gets interesting:


To be honest, not really because it's too entangled, let remove relations and nodes that are not connected between components (like a module in repo depending only on another module inside repo, we don't care about those):

As it's obvious, the repo is tightly coupled to lib RL modules. Most of it is actually on two modules "wikibase" (namespacization) and "util.inherit". Let's remove these two (just moving them to repo for example):

Still tightly coupled.

Also it's worth mentioning that some modules in lib are copied from repo javascript submodules: "jquery.event.special.eachchange", "jquery.ui.suggester", "util.highlightSubstring", "util.inherit" because these files are tightly coupled to those repo files. We have a test to make sure they don't diverge but this is not good.

My suggestion: Rewrite "jquery.wikibase.linkitem" module to use VueJS (specially since mediawiki core now supports VueJS). This would decouple client RL modules from lib modules and we can simply move all RL modules to repo/view and drop all of the copies. This is already tracked on T226976: Convert linkitem module to use OOUI, instead of jQuery UI

Wow, thanks for the report, this is super interesting. Did you use anything to generate the dep graphs?

Also it's worth noting vue2 RL module is being loaded on every wiki but it's not being depended by client at all (it depends on the core's vue module) and it's only needed by WikibaseLexeme. This needs fixing.

T247518: Lexeme: Use "vue" ResourceLoader module from core has a working change attached, if anyone wants to review it. (The other subtasks of T247520: Use "vue" ResourceLoader module from core also need to be fixed before we can get rid of the module.) Though I would say that the module is registered on every client wiki, but not loaded (i. e. we don’t pay the cost of transferring it over the network, only the module definition bloats the startup module a bit.)

Wow, thanks for the report, this is super interesting. Did you use anything to generate the dep graphs?

The startup module has dependency of all RL listed: https://en.wikipedia.org/w/load.php?lang=en&modules=startup&only=scripts&raw=1&skin=vector&debug=true

It was easy to write something that goes through that json data and build this.

import json
import sys
import random
import math
from collections import defaultdict
from subprocess import check_call

import networkx as nx
import matplotlib.pyplot as plt
from networkx.drawing.nx_agraph import graphviz_layout
from networkx.drawing.nx_pydot import write_dot

G = nx.DiGraph()
client_modules = [
    'wikibase.client.init',
    'wikibase.client.data-bridge.init',
    'wikibase.client.data-bridge.externalModifiers',
    'mw.config.values.wbDataBridgeConfig',
    'wikibase.client.data-bridge.app',
    'wikibase.client.data-bridge.app.modern',
    'wikibase.client.data-bridge.app.legacy',
    'wikibase.client.miscStyles',
    'wikibase.client.linkitem.init',
    'jquery.wikibase.linkitem',
    'wikibase.client.action.edit.collapsibleFooter'
]
lib_modules = [
    'mw.config.values.wbSiteDetails',
    'mw.config.values.wbRepo',
#    'wikibase',
    'wikibase.buildErrorOutput',
    'wikibase.sites',
    'wikibase.api.RepoApi',
    'wikibase.api.ValueCaller',
    'vue2',
    'jquery.event.special.eachchange',
    'jquery.ui.suggester',
    'util.highlightSubstring',
#    'util.inherit',
    'jquery.wikibase.siteselector',
    'jquery.wikibase.wbtooltip'
]
repo = []
with open('rl_modules.json', 'r') as f:
    wikibase_modules = json.loads(f.read())
with open(sys.argv[1], 'r') as f:
    modules = json.loads(f.read())
for i in range(len(modules)):
    module = modules[i]
    if module[0] not in wikibase_modules:
        continue
    if module[0] in ['wikibase', 'util.inherit']:
        continue
    if module[0] not in (client_modules + lib_modules):
        repo.append(module[0])
    add_node = False
    if len(module) < 3:
        continue
    for module_dep in module[2]:
        if modules[module_dep][0] in lib_modules:
            G.add_edge(module[0], modules[module_dep][0])
            add_node = True
    if add_node:
        G.add_node(module[0])
print(G.number_of_nodes())
repo = G.subgraph(repo).nodes()
client_modules = G.subgraph(client_modules).nodes()
write_dot(G,'G.dot')
with open('G.dot', 'r') as f:
    G_dot_data = f.read()
G_dot_data = G_dot_data.replace('digraph  {', 'digraph  {' +"""\nsubgraph cluster_0 {
        label="Client";
        \"""" + '"; "'.join(client_modules) + """"
    }

    subgraph cluster_1 {
        label="Lib";
        \"""" + '"; "'.join(lib_modules) + """"
    }

    subgraph cluster_2 {
        label="Repo/View";
        \"""" + '"; "'.join(repo) + """"
    }"""
        
)

with open('G.dot', 'w') as f:
    f.write(G_dot_data)
check_call(['dot','-Tpng','G.dot','-o','G.png'])

This produces the last graph but with some adjustments you can produce anything else.

I originally wrote this for RL terminators and just reused most of the code for this report.

Also it's worth noting vue2 RL module is being loaded on every wiki but it's not being depended by client at all (it depends on the core's vue module) and it's only needed by WikibaseLexeme. This needs fixing.

T247518: Lexeme: Use "vue" ResourceLoader module from core has a working change attached, if anyone wants to review it. (The other subtasks of T247520: Use "vue" ResourceLoader module from core also need to be fixed before we can get rid of the module.) Though I would say that the module is registered on every client wiki, but not loaded (i. e. we don’t pay the cost of transferring it over the network, only the module definition bloats the startup module a bit.)

With the Lexeme change merged, I realized that we can move the vue2 module from Lib to View, avoiding even the startup module cost (except on repo wikis): https://gerrit.wikimedia.org/r/605593

(Of course we should still remove it eventually, but that’ll take a bit more work. Moving it, and thereby removing it from client wikis, is simpler and already an improvement.)

with open('rl_modules.json', 'r') as f:
    wikibase_modules = json.loads(f.read())
with open(sys.argv[1], 'r') as f:
    modules = json.loads(f.read())

I have a couple of questions:

  1. Is this rl-modules.json file handcrafted, or generated by something else?
  2. What kind of json file are you passing into this script? I see that you attached a link to some raw js, but can you please be more specific as to how you got a json file from there? (I suspect it's that giant list in this raw file..)
with open('rl_modules.json', 'r') as f:
    wikibase_modules = json.loads(f.read())
with open(sys.argv[1], 'r') as f:
    modules = json.loads(f.read())

I have a couple of questions:

That's just two. I want more :D

  1. Is this rl-modules.json file handcrafted, or generated by something else?

Yeah I should have somehow mentioned this. This is the list of modules wikibase registers, it can contain even old modules, we use this as a filter all of non-wikibase modules (you can't take modules starting with "wikibase." because lots of modules in wikibase don't start with it. like "util.inherit"). It doesn't need updating, unless we add a new RL module which we barely do.I put it in a gist: https://gist.githubusercontent.com/Ladsgroup/4d58be7207bd55a62487f2605a52f032/raw/e7e81850aa2e6d6f22aec5acd5e50c070e4ecb61/rl_modules.json

  1. What kind of json file are you passing into this script? I see that you attached a link to some raw js, but can you please be more specific as to how you got a json file from there? (I suspect it's that giant list in this raw file..)

That is the output of all RL module dependencies produced by the startup module: https://en.wikipedia.org/w/load.php?lang=en&modules=startup&only=scripts&raw=1&skin=vector&debug=true (Copy the object that goes to mw.loader.register).
I should make it more streamline but feeding javascript to python directly is not something I would find appetizing :D feel free to improve this.

Note that https://en.wikipedia.org/w/load.php?lang=en&modules=startup&only=scripts&raw=1&skin=vector&debug=true will get you the modules *without* repo whereas https://wikidata.org/w/load.php?lang=en&modules=startup&only=scripts&raw=1&skin=vector&debug=true will get it including repo.

The complete graph; including repo has the graphviz as follows:

strict digraph  {
subgraph cluster_0 {
        label="Client";
        "jquery.wikibase.linkitem"
    }

    subgraph cluster_1 {
        label="Lib";
        "mw.config.values.wbSiteDetails"; "mw.config.values.wbRepo"; "wikibase"; "wikibase.buildErrorOutput"; "wikibase.sites"; "wikibase.api.RepoApi"; "wikibase.api.ValueCaller"; "vue2"; "jquery.event.special.eachchange"; "jquery.ui.suggester"; "util.highlightSubstring"; "util.inherit"; "jquery.wikibase.siteselector"; "jquery.wikibase.wbtooltip"
    }

    subgraph cluster_2 {
        label="Repo/View";
        "wikibase.formatters.ApiValueFormatter"; "wikibase.experts.__namespace"; "wikibase.experts.Entity"; "wikibase.entityPage.entityLoaded"; "wikibase.EntityInitializer"; "wikibase.getUserLanguages"; "wikibase.ui.entityViewInit"; "jquery.wikibase.entityselector"; "wikibase.entityChangers.EntityChangersFactory"; "wikibase.utilities.ClaimGuidGenerator"; "wikibase.view.__namespace"; "wikibase.view.ControllerViewFactory"; "jquery.inputautoexpand"; "jquery.ui.commonssuggester"; "jquery.ui.languagesuggester"; "util.ContentLanguages"; "dataValues.DataValue"; "valueFormatters"; "jquery.valueview.Expert"; "jquery.valueview.experts.QuantityInput"; "jquery.valueview.ExpertExtender"; "wikibase.lexeme"; "wikibase.lexeme.lexemeview"; "wikibase.lexeme.special.NewLexeme"
    }
"wikibase.formatters.ApiValueFormatter";
wikibase;
"wikibase.experts.__namespace";
"wikibase.experts.Entity";
"mw.config.values.wbRepo";
"wikibase.entityPage.entityLoaded";
"wikibase.EntityInitializer";
"wikibase.getUserLanguages";
"wikibase.ui.entityViewInit";
"jquery.wikibase.wbtooltip";
"wikibase.api.ValueCaller";
"jquery.wikibase.linkitem";
"jquery.wikibase.siteselector";
"wikibase.api.RepoApi";
"wikibase.sites";
"wikibase.buildErrorOutput";
"mw.config.values.wbSiteDetails";
"util.inherit";
"jquery.ui.suggester";
"jquery.event.special.eachchange";
"util.highlightSubstring";
"jquery.wikibase.entityselector";
"wikibase.entityChangers.EntityChangersFactory";
"wikibase.utilities.ClaimGuidGenerator";
"wikibase.view.__namespace";
"wikibase.view.ControllerViewFactory";
"jquery.inputautoexpand";
"jquery.ui.commonssuggester";
"jquery.ui.languagesuggester";
"util.ContentLanguages";
"dataValues.DataValue";
valueFormatters;
"jquery.valueview.Expert";
"jquery.valueview.experts.QuantityInput";
"jquery.valueview.ExpertExtender";
"wikibase.lexeme";
"wikibase.lexeme.lexemeview";
"wikibase.lexeme.special.NewLexeme";
"wikibase.formatters.ApiValueFormatter" -> wikibase;
"wikibase.experts.__namespace" -> wikibase;
"wikibase.experts.Entity" -> "mw.config.values.wbRepo";
"wikibase.entityPage.entityLoaded" -> wikibase;
"wikibase.EntityInitializer" -> wikibase;
"wikibase.getUserLanguages" -> wikibase;
"wikibase.ui.entityViewInit" -> "jquery.wikibase.wbtooltip";
"wikibase.ui.entityViewInit" -> "wikibase.api.ValueCaller";
"jquery.wikibase.wbtooltip" -> "wikibase.buildErrorOutput";
"jquery.wikibase.linkitem" -> "jquery.wikibase.siteselector";
"jquery.wikibase.linkitem" -> "jquery.wikibase.wbtooltip";
"jquery.wikibase.linkitem" -> "wikibase.api.RepoApi";
"jquery.wikibase.linkitem" -> "wikibase.sites";
"jquery.wikibase.siteselector" -> "jquery.event.special.eachchange";
"jquery.wikibase.siteselector" -> "jquery.ui.suggester";
"jquery.wikibase.siteselector" -> "util.highlightSubstring";
"wikibase.api.RepoApi" -> "util.inherit";
"wikibase.sites" -> "mw.config.values.wbSiteDetails";
"wikibase.buildErrorOutput" -> wikibase;
"jquery.ui.suggester" -> "util.inherit";
"jquery.wikibase.entityselector" -> "jquery.event.special.eachchange";
"jquery.wikibase.entityselector" -> "jquery.ui.suggester";
"wikibase.entityChangers.EntityChangersFactory" -> wikibase;
"wikibase.entityChangers.EntityChangersFactory" -> "wikibase.api.RepoApi";
"wikibase.utilities.ClaimGuidGenerator" -> "util.inherit";
"wikibase.utilities.ClaimGuidGenerator" -> wikibase;
"wikibase.view.__namespace" -> wikibase;
"wikibase.view.ControllerViewFactory" -> "jquery.wikibase.siteselector";
"wikibase.view.ControllerViewFactory" -> "mw.config.values.wbRepo";
"wikibase.view.ControllerViewFactory" -> "wikibase.buildErrorOutput";
"wikibase.view.ControllerViewFactory" -> "wikibase.sites";
"jquery.inputautoexpand" -> "jquery.event.special.eachchange";
"jquery.ui.commonssuggester" -> "jquery.ui.suggester";
"jquery.ui.commonssuggester" -> "util.highlightSubstring";
"jquery.ui.languagesuggester" -> "jquery.ui.suggester";
"util.ContentLanguages" -> "util.inherit";
"dataValues.DataValue" -> "util.inherit";
valueFormatters -> "util.inherit";
"jquery.valueview.Expert" -> "util.inherit";
"jquery.valueview.experts.QuantityInput" -> "jquery.ui.suggester";
"jquery.valueview.ExpertExtender" -> "jquery.event.special.eachchange";
"wikibase.lexeme" -> wikibase;
"wikibase.lexeme.lexemeview" -> "jquery.wikibase.wbtooltip";
"wikibase.lexeme.special.NewLexeme" -> "mw.config.values.wbRepo";
"wikibase.lexeme.special.NewLexeme" -> "wikibase.api.RepoApi";
}

This can be copy pasted to sites like http://www.webgraphviz.com/ and tweaked to look at different things.

The re-summarise what Amir said form this analysis:

  • Client doesn't use lib much at all. Indeed, Client barely has any (non-bridge) RL modules
  • Repo and Lib are highly coupled but given that most of it is *only* used in Repo we could move it out of lib

The re-summarise what Amir said form this analysis:

  • Client doesn't use lib much at all. Indeed, Client barely has any (non-bridge) RL modules
  • Repo and Lib are highly coupled but given that most of it is *only* used in Repo we could move it out of lib

If you remove the repo bits, the dependency of client-lib will turn out to be different. Considering the transitive dependency, one client module actually is depending on every RL module of lib (except vue2 and one or two other) through depending on other lib modules:

Copy over from meeting notes:

SummaryRepo and View heavily depend on one another. Repo also depends on Lib.
Risks, threats, challenges identifiedDecoupling client might break gadgets or user scripts on Client wikis. Keep Product in the loop.
Opportunities noticedOnly one Client feature depends on Lib – suggestion: rewrite in Vue to fully decouple Client front-end.
Other remarks

It was brought up that what's the relationship between View and Repo and I initially thought they are really entangled to each other but after checking, it seems they are pretty contained and repo modules depend on view and view don't depend on repo (except one violation: "mw.config.values.wbRefTabsEnabled"):

Ladsgroup closed this task as Resolved.Jun 24 2020, 10:10 AM