https://bugzilla.wikimedia.org/show_bug.cgi?id=47296

Krinkle <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |UNCONFIRMED
     Ever confirmed|1                           |0

--- Comment #7 from Krinkle <[email protected]> ---
I'm not convinced that this should be solved by suppressing the exception and
turning it into an optional callback.

When a dependency is specified in the module manifest that doesn't exist[1], we
throw. This is currently the case and (afaik) acceptable behaviour because it
is a fatal problem. The code must not be attempted to run without that other
module.

When a dependency is specified inline at runtime (wrapping the code in a
function) which doesn't exist, we throw. I don't see why that would be any
different.

Yes, mw.loader.using has an error callback. But aside from this being almost
never used (I haven't seen it in use, ever), it is unlikely that an
implementation would be anything other than throwing an error. The module ought
to be there.

When a module is added to the page by an extension or by core, it is with the
intention to make something happen on that page. If that module is unable to
load its code, that should throw.

ext.wikiLove.init, ext.articleFeedback.init, VisualEditor,
mediawiki.page.ready. All of these load modules after a check or event. They
should not be loaded on the page if their dependencies are not available, and
loading it anyway should throw.

I'd recommend: Look at this error and fix it.

In this case it means a module is loaded on mobile but one of its dependencies
is not yet compatible with mobile. That means it (=jquery.tablesorter) should
either be made compatible, or the containing module removed from mobile for the
mean while (=mediawiki.page.ready). This isn't my own idea, I've seen this
happen before with various other modules and I'm merely suggesting we act the
same.

This error is good, because without it you wouldn't have known that we made a
mistake and forgot one of the dependencies of mediawiki.page.ready when you
enabled it on mobile. This is how we found them before as well, except before
they were server-side dependencies and now they're inline.


Having said that, I'd like a view from Roan and/or Trevor on this as well.
Personally I'd like to keep this exception and instead deprecate the error
callback.


[1] Exist as in, in the current execution. Because since the 'target' property
was created a module can exist in general, but be invisible to the current
request context based on the 'target' parameter, thus making any errors related
to it somewhat ambiguous (not visible in this target vs. module not existing in
general - very different problems).

-- 
You are receiving this mail because:
You are on the CC list for the bug.
_______________________________________________
Wikibugs-l mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l

Reply via email to