Page MenuHomePhabricator

Merge single use VisualEditor ResourceLoader modules to reduce module count
Open, Needs TriagePublic

Description

Partially related to T107283 (T107283#5002772), but that deals with avoiding duplication in defining the modules, and this is about merging some extension modules without changing anything in the VisualEditor library modules.json

Per codesearch, the following modules are only used once, as a dependency of another module, and can be merged:

  • socket.io -> ext.visualEditor.rebase
  • dompurify -> ext.visualEditor.sanitize
  • color-picker -> ext.visualEditor.rebase
  • papaparse -> ext.visualEditor.core
  • spark-md5 -> ext.visualEditor.core
  • ext.visualEditor.progressBarWidget -> ext.visualEditor.desktopArticleTarget.init
  • ext.visualEditor.tempWikitextEditorWidget -> ext.visualEditor.desktopArticleTarget.init
  • ext.visualEditor.core.utils -> ext.visualEditor.base
  • ext.visualEditor.commentAnnotation -> ext.visualEditor.rebase
  • ext.visualEditor.rebase -> ext.visualEditor.collabTarget
  • ext.visualEditor.mwformatting -> ext.visualEditor.mwextensions
  • treeDiffer -> ext.visualEditor.diffing
  • diffMatchPatch -> ext.visualEditor.diffing
  • ext.visualEditor.checkList -> ext.visualEditor.rebase
  • ext.visualEditor.diffing -> ext.visualEditor.mwcore
  • ext.visualEditor.mwlanguage -> ext.visualEditor.mwextensions
  • ext.visualEditor.mwalienextension -> ext.visualEditor.mwextensions
  • ext.visualEditor.mwgallery -> ext.visualEditor.mwextensions

(Being a QUnit dependency does not count)

Note however that some of the dependencies have no ResourceLoader "group" specified, while the module that uses them is in the group "visualEditorA" - not sure if that affects the ability to merge them. We can start with only merging within the visualEditorA group.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 722465 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/extensions/VisualEditor@master] Merge some modules into `ext.visualEditor.mwextensions`

https://gerrit.wikimedia.org/r/722465

Although it is a (minor) performance improvement, we don't really want to do this, because it makes the code (a little) harder to maintain. We have some long dependency chains in the VisualEditor code, and it's easier to avoid mistakes when they're expressed as dependencies between ResourceLoader modules, than when they're maintained by carefully placing the files in the correct order.

Reducing the number of modules used by VisualEditor has came up in the past. I can't find where it was discussed (maybe it was some meeting where no one took notes… sorry), but I remember that we created T225842 as a possible approach to resolving the problem – reducing the module count without losing the clear dependencies.

Although it is a (minor) performance improvement, we don't really want to do this, because it makes the code (a little) harder to maintain. We have some long dependency chains in the VisualEditor code, and it's easier to avoid mistakes when they're expressed as dependencies between ResourceLoader modules, than when they're maintained by carefully placing the files in the correct order.

Reducing the number of modules used by VisualEditor has came up in the past. I can't find where it was discussed (maybe it was some meeting where no one took notes… sorry), but I remember that we created T225842 as a possible approach to resolving the problem – reducing the module count without losing the clear dependencies.

This makes sense, and would be solved by private modules, but until then, each module adds to the registry size loaded on all pages, which I've been trying to help reduce in my free time. What if it were possible to add some inline documentation? Like have resource loader ignore any messages that start with @, and then in the module definition you can document

"mesages": [
	"@these messages are for the (feature) code",
	"message-one",
	"message-two",
	"@these messages are for the (other feature) code",
	"message-three",
	"message-four"
]

and similar for maybe scripts, styles, dependencies - would that help?

I mean, that's just the same thing with extra steps…

I mean, that's just the same thing with extra steps…

My understanding of the issue was that there were concerns about making it harder to follow dependencies when its done via files in a certain order within the module definition, this would fix the potential confusion by allowing documentation while also reducing the performance impact of having so many modules