Page MenuHomePhabricator

Organise module files in directories based on their module name
Open, NormalPublic

Description

This is a bit of clean-up for a couple reasons.

Finding files

Make it easier to find files in the repository based on their module name. This applies to features like "Find File" in text editors (eg. +P / ctrl+P in Atom and Sublime Text), as well as to repository viewers (eg. T on GitHub).

Contributing

Even in our current core stack (with good ol' ResourceLoader concatenating files), there's a fair amount of complexity that can overwhelm new contributors. And even for experienced contributors, we don't all work with all files all the time. So being able to easily tell which (other) files load as part of a given module is quite useful.

I would expect this to reduce overall confusion, but also reduce risk of forgetting to declare dependencies between related code paths (given files in the same directory but part of separate modules). It would also make it easier to create something in less feedback cycles of "try this", "get an error" (eg. for cases where we do detect something is broken, missing dependencies tends to not be one of them, though).

Past

Some past work has been put in towards this, but so far only in certain sub-trees. Never across the board.

Some past conversions in resources/src/

Also, we've been doing fairly well in resources/lib/ where every module has its own directory now, yay!

Future

This will also get things in a better shape of the future that awaits us in T133462, where it'll likely become a prerequisite for a module to either be one file, or be its own directory. Even if we don't enforce that, it'd be even more confusing than today if we have files importing each other using require('./example'); but also have files in the same directory that aren't reachable as part of the module.

Proposal

The proposal is the following two mandates:

Every file shipped in a ResourceLoader module must be in resources/ as either a single file containing the module name in its file name, or in a (sub) directory named after the module.
For modules that have their own directory, there must not be files in the same directory that are part of a different module.
Currently fine as-is
  • "jquery.chosen" is at resources/lib/jquery.chosen/
  • "moment" is at resources/lib/moment/
  • "qunit" is at resources/lib/qunitjs/
  • "jquery.tipsy" is at resources/src/jquery.tipsy/
  • "mediawiki.toolbar" is at resources/src/mediawiki.toolbar/
  • "mediawiki.widgets.datetime" is at resources/src/mediawiki.widgets.datetime/
  • "startup" is at resources/src/startup.js
Examples to change
  • resources/lib/jquery/

This directory contains things loosely related to "jQuery" in some way or another, registered as multiple different modules. The ones that are single-file modules could be moved up to be in lib/ directory. The others should get their own directory. Leaving jquery/ to only contain jquery.js and jquery.migrate.js – the contents of the module currently registered as "jquery".

  • resources/src/mediawiki/
    • api.js
    • api/
      • category.js
    • htmlform/
    • page/
      • ready.js
      • startup.js
      • watch.js

This the "api" and "page" are remnants of my attempt to make the directory structure reflect the run-time class hierarchy (instead of the module/bundle structure). This is also predates things like module.exports.

The idea was to roughly be able to translate a public identifier in JavaScript (e.g. mw.Api.prototype.isCategory or mw.page.watch.updateLinks()) to a path on the file system. I didn't think about this well, and I regret doing it. It didn't work because the way we namespace our modules is by module name, not my namespace hierarchy. Even in the above examples we can see a few clear flaws and limitations:

  • mediawiki/api/category.js does not translate to mw.api.category = {}. Rather it translates to the module name mediawiki.api.category. The actual identifier is mw.Api and several methods such as mw.Api#isCategory.
  • mediawiki/page/watch.js - This was at some point the first and only module to justify my (bad) idea. We did have mw.page.watch. It existed. But it doesn't exist anymore. And the other files in the directory mostly don't have any public identifiers. They're just run-time logic with their file reflecting the module names: mediawiki.page.startup, mediawiki.page.ready, etc.

Given that all of these are single-file modules, there are two ways we could solve them:

  1. Put them directly in resources/src/ as mediawiki.api.category.js and mediawiki.page.ready.js.
  2. Give some of the related modules their own shared directory, e.g. resources/src/mediawiki.api/ and resources/src/mediawiki.page/, with the single-file modules living inside of it.

Both of these would satisfy the two proposed mandates.

Progress

Below listing is based on the structure as it was when we started.

Reference: https://gerrit.wikimedia.org/g/mediawiki/core/+/42956b99b558294e5aa8c6c0e2a8f18513478022/resources/

Items are checked off when the file or directory has been checked recursively to match the above two principles (and renamed or split as needed).

resources/lib/

  • CLDRPluralRuleParser/
  • html5shiv/
  • jquery/
  • jquery.chosen/
  • jquery.client/
  • jquery.i18n/
  • jquery.ui/
  • moment/
  • mustache/
  • oojs-router/
  • oojs-ui/
  • oojs/
  • qunitjs/
  • sinonjs/

resources/src/

  • jquery.tipsy/
  • jquery/
    • jquery.byteLength
    • jquery.color
    • jquery.confirmable
    • jquery.hidpi (removed)
    • jquery.lengthLength
    • jquery.makeCollapsible
    • jquery.farbtastic
    • jquery.tablesorter
    • jquery.spinner
    • (misc)
  • mediawiki.action/
  • mediawiki.hidpi-skip.js
  • mediawiki.language/
  • mediawiki.legacy/
  • mediawiki.less/ (less importdir: not a module)
  • mediawiki.libs.jpegmeta/
  • mediawiki.libs.pluralruleparser/
  • mediawiki.messagePoster/
  • mediawiki.rcfilters/
  • mediawiki.router/
  • mediawiki.skinning/
  • mediawiki.special/
  • mediawiki.toolbar/
  • mediawiki.ui/
  • mediawiki.widgets.datetime/
  • mediawiki.widgets.visibleLengthLimit/
  • mediawiki.widgets/
  • mediawiki/
    • mediawiki/api/
    • mediawiki/htmlform/
    • mediawiki/page/
  • moment/
  • oojs-global.js
  • oojs-ui-local.js
  • startup.js

Follow-up

  • Consider merging 'mediawiki.ForeignApi' and 'mediawiki.ForeignApi.core.'.
  • Consider merging 'mediawiki.page.ready' and 'mediawiki.page.startup'.

See T192623 further about these.

Element.js
// FIXME: mw.htmlform.Checker also sets this to empty object
mw.htmlform = {};
Checker.js
// FIXME: mw.htmlform.Element also sets this to empty object
mw.htmlform = {};

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 432122 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] mediawiki.notification: Move files to their own directory

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

Change 432123 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] mediawiki.messagePoster: Move WikitextMessagePoster.js to its own directory

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

Krinkle updated the task description. (Show Details)May 9 2018, 5:21 PM

Change 432123 merged by jenkins-bot:
[mediawiki/core@master] mediawiki.messagePoster: Move WikitextMessagePoster.js to its own directory

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

Change 432132 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] build: Convert jsduck.json whitelist to blacklist

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

Change 432134 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resources: Move various single-file mediawiki.* modules to src/

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

Change 432122 merged by jenkins-bot:
[mediawiki/core@master] mediawiki.notification: Move files to their own directory

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

Change 432132 merged by jenkins-bot:
[mediawiki/core@master] build: Convert jsduck.json whitelist to blacklist

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

Change 432134 merged by jenkins-bot:
[mediawiki/core@master] resources: Move various single-file mediawiki.* modules to src/

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

Change 432151 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resources: Move more various single-file mediawiki.* modules to src/

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

Change 432152 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] mediawiki.htmlform: Move files to their own module directory

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

Change 432155 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] mediawiki.page.gallery.styles: Move files to src/

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

Change 432151 merged by jenkins-bot:
[mediawiki/core@master] resources: Move more various single-file mediawiki.* modules to src/

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

Change 432155 merged by jenkins-bot:
[mediawiki/core@master] mediawiki.page.gallery.styles: Move files to src/

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

Change 432152 merged by jenkins-bot:
[mediawiki/core@master] mediawiki.htmlform: Move files to their own module directory

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

Change 432299 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resources: Move the remaining src/mediawiki/ files

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

Krinkle updated the task description. (Show Details)May 9 2018, 10:17 PM

Change 432299 merged by jenkins-bot:
[mediawiki/core@master] resources: Move the remaining src/mediawiki/ files

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

Krinkle updated the task description. (Show Details)May 10 2018, 5:18 PM

Change 432407 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] [WIP] resources: Give mediawiki.special.* files their own place in src/

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

Krinkle updated the task description. (Show Details)May 10 2018, 5:58 PM

Change 432407 merged by jenkins-bot:
[mediawiki/core@master] resources: Give mediawiki.special.* files their own place in src/

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

Od1n added a subscriber: Od1n.May 12 2018, 6:34 PM

I have just updated a few links, there are probably more remaining.

Krinkle updated the task description. (Show Details)
Krinkle updated the task description. (Show Details)May 14 2018, 12:21 AM
Od1n removed a subscriber: Od1n.May 14 2018, 1:34 AM

@Krinkle all of this is great work, thanks for leading it. I wanted to help with rcfilters, but if I go by the standard you put up, it seems it's already organized into its own folder anyways. We already organized the files inside based on logical structure (dm/ui/styles etc) so I'm trying to see if I'm missing something. I mainly ask since it's in the TODO list in the description, so I want to verify in case you want to rearrange something or if they're "passing" by default...

Krinkle added a comment.EditedMay 16 2018, 10:44 PM

@Mooeypoo Yeah, mediawiki.rcfilters/ is definitely not unorganised. But, it is organised differently than what we're proposing for other modules.

Specifically, RCFilters currently has its directories match the JS namespaces. This in itself could be fine, except that we want to follow the module structure instead, and RCFilters' module structure does not match its JS namespaces. Its stylesheets and images also use directories shared by multiple modules.

I'll describe one example to get you started, then we can figure out the rest on Gerrit?

The files declared as part of mediawiki.rcfilters.filters.ui do not currently share a common parent directory that isn't shared by other modules. Its JavaScript files in src/mediawiki.rcfilters/ui/ are fine, but it has two issues. Its various stylesheets are directly in src/mediawiki.rcfilters/styles/ which is shared by other modules. So the minimalist change would be to move those to src/mediawiki.rcfilters/ui/ or src/mediawiki.rcfilters/ui/styles/. It also has two JavaScript files declared that aren't in the /ui/ subdirectory (mw.rcfilters.HighlightColors.js and mw.rcfilters.init.js).

Krinkle updated the task description. (Show Details)May 16 2018, 10:46 PM
Krinkle updated the task description. (Show Details)May 16 2018, 10:51 PM

Change 433492 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] jquery.spinner: Move files to their own src/ directory

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

Change 433492 merged by jenkins-bot:
[mediawiki/core@master] jquery.spinner: Move files to their own src/ directory

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

Krinkle updated the task description. (Show Details)May 22 2018, 12:16 PM

Change 434735 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] Remove the unused 'jquery.farbtastic' module

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

Krinkle updated the task description. (Show Details)May 23 2018, 4:42 PM

Change 434735 merged by jenkins-bot:
[mediawiki/core@master] Remove the unused 'jquery.farbtastic' module

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

Krinkle removed Krinkle as the assignee of this task.Jun 24 2018, 1:47 AM
Krinkle edited projects, added Performance-Team (Radar); removed Performance-Team.
Krinkle moved this task from Limbo to Perf issue on the Performance-Team (Radar) board.
Vvjjkkii renamed this task from Organise module files in directories based on their module name to 9mdaaaaaaa.Jul 1 2018, 1:11 AM
Vvjjkkii raised the priority of this task from Normal to High.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed subscribers: gerritbot, Aklapper.
Krinkle renamed this task from 9mdaaaaaaa to Organise module files in directories based on their module name.Jul 1 2018, 10:58 PM
Krinkle lowered the priority of this task from High to Normal.
Krinkle updated the task description. (Show Details)
Krinkle added a subscriber: Aklapper.

Change 443340 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/extensions/MultimediaViewer@master] Move files to their own directory per module

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

Change 443340 merged by jenkins-bot:
[mediawiki/extensions/MultimediaViewer@master] Move files to their own directory per module

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

Change 450598 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/extensions/EventLogging@master] Move files to their own directory per module

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

Change 450598 merged by jenkins-bot:
[mediawiki/extensions/EventLogging@master] Move files to their own directory per module

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

Change 453876 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resources: Move non-jquery files from /resources/lib/jquery to /resources/lib

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

Change 453876 merged by jenkins-bot:
[mediawiki/core@master] resources: Move non-jquery files from /resources/lib/jquery to /resources/lib

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

Change 494047 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] mediawiki.cldr: Move file to its own directory

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

Change 494047 merged by jenkins-bot:
[mediawiki/core@master] mediawiki.cldr: Move file to its own directory

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

Change 506232 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/extensions/CentralNotice@master] ext.centralNotice.startUp: Convert to packageFiles module

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

Change 506232 merged by jenkins-bot:
[mediawiki/extensions/CentralNotice@master] ext.centralNotice.startUp: Convert to packageFiles module

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

Change 510584 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/extensions/CentralNotice@master] ext.centralNotice.kvStore: Move to its own directory

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

Change 510584 merged by jenkins-bot:
[mediawiki/extensions/CentralNotice@master] ext.centralNotice.kvStore: Move to its own directory

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

Change 517109 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] specials: Combine small miscellaneous 'mediawiki.special.*' modules

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

Change 517109 merged by jenkins-bot:
[mediawiki/core@master] specials: Combine small miscellaneous 'mediawiki.special.*' modules

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