Page MenuHomePhabricator
Create Task
Maniphest T146748

Lint JS sources dependency graph against ResourceModules configuration
Closed, DeclinedPublic
Actions

Description

Intro

Currently ResourceModules in extension.json is the source of truth of the JS dependency graph and which assets will be served.

At the same time, the JS sources explicitly define dependencies on other modules by using them in the source or requireing them, giving form to a parallel dependency graph specified by the source code.

Such source-defined dependencies may get out of sync or be mis-configured with the separate ResourceModules configuration, giving place to ResourceLoader serving assets with order/dependencies that are not really the constraints defined by the source files.

This results in non-deterministic JS exceptions in the runtime, missing messages at runtime, bad dependency order in ResourceModules configuration, etc.

Some examples: T146031 T145566 T99461

What

We want a linter that parses the frontend sources of extensions and builds a dependency graph from the source, and then checks that those constraints specified by the sources are reflected in extension.json ResourceModules.

That way, extension.json ResourceModules will for sure reflect the needs of the source code. We want that linter to run on CI so that no sources on master are out of sync with extension.json config.

Things to be linted (current = current js file, current-RLmodules = ResourceModules that include current in scripts):

  • Messages in current are in current-RLmodules
  • Required modules/function/variables in current are somewhere in the dependencies of current-RLmodules
    • For legacy M.require/M.define
    • For RL new require/module.exports
    • For global JS variables in mw (mw.blah.blah)
  • Templates used in current are in current-RLmodules

How

Please, help review

  • Nothing right now

Details

Show related patches Customize query in gerrit

Related Objects
Search...

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 27 2016, 10:31 AM
• Jhernandez triaged this task as Medium priority.Sep 27 2016, 10:31 AM
• Jhernandez moved this task from Incoming to Epics/Goals on the Web-Team-Backlog board.
gerritbot subscribed.Sep 27 2016, 10:34 AM

Change 312986 had a related patch set uploaded (by Hashar):
[MobileFrontend] exp job for npm run lint:modules

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

gerritbot added a project: Patch-For-Review.Sep 27 2016, 10:34 AM
gerritbot added a comment.Sep 27 2016, 10:38 AM

Change 312986 merged by jenkins-bot:
[MobileFrontend] exp job for npm run lint:modules

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

gerritbot added a comment.Sep 27 2016, 10:48 AM

Change 312850 had a related patch set uploaded (by Jhernandez):
WIP: Add resource modules linter

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

gerritbot added a comment.Sep 27 2016, 10:48 AM

Change 312988 had a related patch set uploaded (by Jhernandez):
Avoid calling mw.msg with variables

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

• Jhernandez updated the task description. (Show Details)Sep 27 2016, 10:48 AM
• Jhernandez updated the task description. (Show Details)Sep 27 2016, 10:56 AM
• Jhernandez updated the task description. (Show Details)Sep 27 2016, 11:02 AM
gerritbot added a comment.Sep 27 2016, 11:21 AM

Change 312991 had a related patch set uploaded (by Jhernandez):
Make Nearby getDistance test diff readable

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

phuedx awarded a token.Sep 27 2016, 12:14 PM
gerritbot added a comment.Sep 27 2016, 1:40 PM

Change 313008 had a related patch set uploaded (by Jhernandez):
Make loadModule use string literals instead of variables

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

gerritbot added a comment.Sep 27 2016, 1:40 PM

Change 313010 had a related patch set uploaded (by Jhernandez):
Remove dead code CommonsCategoryOverlay

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

gerritbot added a comment.Sep 27 2016, 2:05 PM

Change 313011 had a related patch set uploaded (by Jhernandez):
Add messages used in sources to the modules where they are used

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

gerritbot added a comment.Sep 27 2016, 2:05 PM

Change 313012 had a related patch set uploaded (by Jhernandez):
Import mw.ForeignApi to local scope instead of directly using it

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

gerritbot added a comment.Sep 27 2016, 2:24 PM

Change 313015 had a related patch set uploaded (by Jhernandez):
Move messages in config to where they are use in the source

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

gerritbot added a comment.Sep 27 2016, 8:42 PM

Change 313009 had a related patch set uploaded (by Jhernandez):
Move mobile.browser/browser to mobile.browser/Browser#getSingleton

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

gerritbot added a comment.Sep 28 2016, 10:05 AM

Change 312991 merged by jenkins-bot:
Make Nearby getDistance test diff readable

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

gerritbot added a comment.Sep 28 2016, 10:05 AM

Change 312988 merged by jenkins-bot:
Avoid calling mw.msg with variables

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

gerritbot added a comment.Sep 28 2016, 10:06 AM

Change 313008 merged by jenkins-bot:
Make loadModule use string literals instead of variables

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

gerritbot added a comment.Sep 28 2016, 10:06 AM

Change 313009 merged by jenkins-bot:
Move mobile.browser/browser to mobile.browser/Browser#getSingleton

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

ReleaseTaggerBot added a project: MW-1.28-release (WMF-deploy-2016-10-04_(1.28.0-wmf.21)).Sep 28 2016, 11:00 AM
gerritbot added a comment.Sep 28 2016, 12:11 PM

Change 313010 merged by jenkins-bot:
Remove dead code CommonsCategoryOverlay

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

gerritbot added a comment.Sep 28 2016, 12:22 PM

Change 313015 merged by jenkins-bot:
Move messages in config to where they are use in the source

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

gerritbot added a comment.Sep 28 2016, 12:24 PM

Change 313012 merged by jenkins-bot:
Import mw.ForeignApi to local scope instead of directly using it

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

gerritbot added a comment.Sep 28 2016, 12:54 PM

Change 313205 had a related patch set uploaded (by Jhernandez):
Move mobile-frontend-editor-anon to where it is used

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

gerritbot added a comment.Sep 28 2016, 1:10 PM

Change 313011 merged by jenkins-bot:
Add messages used in sources to the modules where they are used

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

• Jhernandez updated the task description. (Show Details)Sep 28 2016, 3:21 PM
• Jhernandez updated the task description. (Show Details)
gerritbot added a comment.Sep 28 2016, 6:59 PM

Change 313205 merged by jenkins-bot:
Move mobile-frontend-editor-anon to where it is used

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

gerritbot added a comment.Oct 4 2016, 10:09 AM

Change 313980 had a related patch set uploaded (by Jhernandez):
Make mobile.drawers depend on mobile-frontend-overlay-close

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

• Jhernandez updated the task description. (Show Details)Oct 4 2016, 10:12 AM
gerritbot added a comment.Oct 5 2016, 12:00 AM

Change 313980 merged by jenkins-bot:
Make mobile.drawers depend on mobile-frontend-overlay-close

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

ReleaseTaggerBot added a project: MW-1.28-release (WMF-deploy-2016-10-11_(1.28.0-wmf.22)).Oct 5 2016, 12:01 AM
• Jhernandez updated the task description. (Show Details)Oct 5 2016, 9:52 AM
• bmansurov subscribed.Oct 21 2016, 7:49 PM

This work is great. Thanks for pushing it, everyone who is involved. @Jhernandez let me know how I can help with getting this running for all of our patches in all extensions that we maintain.

Jdlrobson changed the task status from Open to Stalled.Nov 16 2016, 5:05 PM
Jdlrobson subscribed.

waiting for feedback from @Jhernandez on https://gerrit.wikimedia.org/r/312850

• Jhernandez changed the task status from Stalled to Open.Mar 6 2017, 12:47 PM

I'm back to working on this. Seems pretty stable now, see https://gerrit.wikimedia.org/r/#/c/312850/18

I sent an email with a question about a warning in MobileFrontend, I'll come back with the answer.

• Jhernandez added a comment.EditedMar 7 2017, 9:09 AM

We've chatted and the next step is

That way we can disable parsing of parts of the code and avoid error reporting in special situations like in [MobileFrontend/resources/mobile.editor.overlay/EditorOverlay.js#L165](https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/resources/mobile.editor.overlay/EditorOverlay.js#L165) where 'visualeditor-mweditmode-tooltip' is used but not defined in module config because it is defined in another extension.

Jdlrobson moved this task from Epics/Goals to Upcoming on the Web-Team-Backlog board.Mar 8 2017, 3:36 PM
Jdlrobson added a project: Reading-Web-Sprint-94.Mar 8 2017, 7:35 PM

^ Since Joaquin said he might want to work on this next sprint let's identify what that work is and attempt to schedule it.

• Jhernandez updated the task description. (Show Details)Mar 9 2017, 11:25 AM
• Jhernandez updated the task description. (Show Details)
• Jhernandez updated the task description. (Show Details)
• Jhernandez mentioned this in T160056: Add disabling comment syntax to resource-modules linter.Mar 9 2017, 11:32 AM
• Jhernandez created subtask T160056: Add disabling comment syntax to resource-modules linter.
• Jhernandez updated the task description. (Show Details)
• Jhernandez removed a project: Reading-Web-Sprint-94.Mar 9 2017, 11:34 AM

This is the epic, I've extracted T160056: Add disabling comment syntax to resource-modules linter and added that to 94 instead.

Jdlrobson removed projects: MW-1.28-release (WMF-deploy-2016-10-11_(1.28.0-wmf.22)), Patch-For-Review.Mar 17 2017, 5:03 PM
• Jhernandez updated the task description. (Show Details)Apr 7 2017, 5:30 PM
• Jhernandez added a comment.Apr 7 2017, 5:34 PM

eb201e4: Add resource modules linter is ready for review.

Last release of the linter: v1.0.15...v1.0.17 (adds disabling via comment syntax)

All linting problems have been fixed on MobileFrontend, right now. @Jdlrobson I'd appreciate some review.

After merging this the followup for the task is talking to Antoine about making it a stable job for MobileFrontend, and maybe seeing remaining issues in other extensions like QuickSurveys, RelatedArticles, etc.

gerritbot added a comment.Apr 7 2017, 7:31 PM

Change 347051 had a related patch set uploaded (by Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Lint modules as part of npm-node-6-jessie

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

gerritbot added a project: Patch-For-Review.Apr 7 2017, 7:31 PM
Jdlrobson removed a project: Patch-For-Review.Apr 7 2017, 7:32 PM
gerritbot added a comment.Apr 7 2017, 7:40 PM

Change 312850 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Add resource modules linter

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

gerritbot added a comment.Apr 7 2017, 7:58 PM

Change 347051 abandoned by Jdlrobson:
Lint modules as part of npm-node-6-jessie

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

ReleaseTaggerBot added a project: MW-1.29-release (WMF-deploy-2017-04-11_(1.29.0-wmf.20)).Apr 7 2017, 8:00 PM
• Jhernandez added a comment.Apr 10 2017, 11:25 AM

@Jdlrobson Re: ^ linting on the npm job: The reason why it is a special job and not an npm one is because for linting we need the whole source tree (core and extension) and in the npm job only the extension is cloned :(

phuedx subscribed.Apr 13 2017, 5:03 AM
In T146748#3085201, @Jdlrobson wrote:

^ Since Joaquin said he might want to work on this next sprint let's identify what that work is and attempt to schedule it.

Was this effective? In my head this remains a 10% project and these tasks are more for capturing thoughts/representing the size of this (awesome) undertaking.

phuedx moved this task from Upcoming to Epics/Goals on the Web-Team-Backlog board.Apr 13 2017, 5:04 AM

I'll move this back to Epics in the meantime as T160056: Add disabling comment syntax to resource-modules linter, for example, is in Sprint +1.

Jdlrobson added a comment.Apr 17 2017, 4:16 PM

We got half of that task done so I'd say yes.
It's a 10% project but it's something the group has identified as being useful. Interest in 10% projects fizzles out naturally and I see it as our duty as a team to help see good ideas come to full fruition. I want to at least wrap up this work for MobileFrontend so we can evaluate it's usefulness.

• Jhernandez closed subtask T160056: Add disabling comment syntax to resource-modules linter as Resolved.Apr 26 2017, 10:20 AM
• Jhernandez mentioned this in T163882: Migrate experimental CI job mwext-MobileFrontend-npm-run-lint-modules to stable.Apr 26 2017, 10:39 AM
• Jhernandez created subtask T163882: Migrate experimental CI job mwext-MobileFrontend-npm-run-lint-modules to stable.
• Jhernandez updated the task description. (Show Details)Apr 26 2017, 10:43 AM
Jdlrobson raised the priority of this task from Medium to High.Apr 26 2017, 3:21 PM

To reflect the fact we are close to finishing this.

• Jhernandez closed subtask T163882: Migrate experimental CI job mwext-MobileFrontend-npm-run-lint-modules to stable as Resolved.May 23 2017, 4:26 PM
Jdlrobson added a comment.May 23 2017, 5:39 PM

@Jhernandez thanks for all your work here! :) Would it make sense to do this for other extensions too?

Jdlrobson updated the task description. (Show Details)May 23 2017, 5:39 PM
hashar subscribed.May 24 2017, 1:08 PM

Well done!! From a quick chat I had with @Jdlrobson , it would be rather nice to have that RL linter to be generalized so it can benefits other extensions. Maybe the linter could be upstreamed to mediawiki/core ?

Krinkle removed projects: MW-1.29-release (WMF-deploy-2017-04-11_(1.29.0-wmf.20)), MW-1.28-release (WMF-deploy-2016-10-04_(1.28.0-wmf.21)).May 25 2017, 2:21 PM
• Jhernandez added a comment.May 25 2017, 3:30 PM

That would be very interesting. As next steps, as seen on the description, I want to run it on more extensions and fix issues that come up. I'll see what I can tackle next.

• Jhernandez updated the task description. (Show Details)May 25 2017, 3:32 PM
Jdlrobson lowered the priority of this task from High to Low.Jun 2 2017, 8:23 PM
• bmansurov unsubscribed.Dec 22 2017, 9:49 PM
Jdlrobson closed this task as Declined.Aug 6 2019, 3:43 PM

I think now we are using Webpack, this is not the direction we are taking.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits