Improve error message for using package require() in module without packageFiles
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Daimona
	Feb 19 2025, 3:26 PM

Description

If I define a QUnit RL module like so:

"QUnitTestModule": {
	"localBasePath": "",
	"remoteExtPath": "CampaignEvents",
	"scripts": [
		"tests/qunit/index.js"
	]
},

And then, in tests/qunit/index.js, I have

'use strict';

require( './foo.test.js' );

The test runner will fail with a nonsensical error message:

Chrome Headless 133.0.0.0 (Linux x86_64) LOG: 'Exception in module-execute in module test.CampaignEvents:'
Chrome Headless 133.0.0.0 (Linux x86_64) WARN: Error: Module "." is not loaded
Error: Module "." is not loaded
    at require (http://localhost:8080/w2/index.php?title=Special:JavaScriptTest/qunit/export&component=CampaignEvents:19:962)
    at http://localhost:8080/w2/load.php?modules=jquery.client%7Cmediawiki.String%2CTitle%2Capi%2Ccldr%2Ccookie%2CjqueryMsg%2Clanguage%2Cqunit-testrunner%2Cuser%2Cutil%7Cmediawiki.libs.pluralruleparser%7Cmediawiki.page.ready%7Csinonjs%7Ctest.CampaignEvents&version=qg4q0:270:109
    at runScript (http://localhost:8080/w2/index.php?title=Special:JavaScriptTest/qunit/export&component=CampaignEvents:11:580)
    at execute (http://localhost:8080/w2/index.php?title=Special:JavaScriptTest/qunit/export&component=CampaignEvents:13:107)
    at doPropagation (http://localhost:8080/w2/index.php?title=Special:JavaScriptTest/qunit/export&component=CampaignEvents:5:920)

The problem is that I'm using require in a module that uses scripts instead of packageFiles, but saying that Module "." is not loaded is arguably not the best way to let a developer know.

Details

Related Changes in Gerrit:

	Subject	Repo	Branch	Lines +/-
	ResourceLoader: Reject module names starting with "./" && "../"	mediawiki/core	master	+10 -1
	ResourceLoader: Improve error when including file in non-package module	mediawiki/core	master	+11 -0

Customize query in gerrit

Event Timeline

Daimona created this task.Feb 19 2025, 3:26 PM

Restricted Application added a project: MediaWiki-Platform-Team. · View Herald TranscriptFeb 19 2025, 3:26 PM

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

Daimona moved this task from Inbox to QUnit on the MediaWiki-Core-Tests board.Feb 19 2025, 3:27 PM

The require() function roughly works as follows:

Script module require(arg):
- -> mw.loader.require(arg)
Package module require(arg):
- (If path starting with ./) Import a file in the current package, or error.
- -> mw.loader.require(arg)
mw.loader.require(arg):
- (If in QUnit mode, look for @non-slash-vendor/non-slash-module or non-slash-name as module name, and then <module>/<path>) Import a private file from the named module, or error.
- Treat arg as module name, and import the public entrypoint from the named module, or error.

In your case the module is a script module, thus its require function would not handle ./.

Script modules may import public modules (in prod), or (in unit tests) may also import private files from other modules.

Given that . is technically a valid module reference (to mean current module), it is correctly parsed as such. But, since there is no current package, it ends up looking for a registered module named ..

I suggest we fix this by explicily rejecting module name . in mw.loader.require() because it is only valid to refer to the "current" module in a package file module, which will have its own dedicated require() function. Via that public lookup, only registered module names can be imported.

(For next triage meeting) This may be a good task for @Hokwelum!

Krinkle added a subscriber: • Hokwelum.Feb 25 2025, 8:19 PM

Thanks for the explanation, and your proposal seems reasonable to me. I think it would also make sense to reject .., otherwise you would get the nonsensical error by doing require( '../foo.test.js' );. Or actually, do we really need to support (back)slashes in RL module names? Unless there is a use case that I'm not aware of, I think it would just contribute to the ambiguity with file paths.

Krinkle renamed this task from Nonsensical error message in QUnit when trying to require files without using packageScripts to Improve error message for using package require() in module without packageFiles.Mar 3 2025, 8:01 AM

Krinkle triaged this task as Low priority.Mar 8 2025, 2:08 AM

Krinkle moved this task from Inbox to Accepted Enhancement on the MediaWiki-ResourceLoader board.

Krinkle moved this task from Inbox, needs triage to Not planned (Patches welcome!) on the MediaWiki-Platform-Team board.Mar 10 2025, 3:27 PM

• Hokwelum claimed this task.Mar 24 2025, 3:40 PM

Change #1130717 had a related patch set uploaded (by Hokwelum; author: Hokwelum):

[mediawiki/core@master] Script modules should not start with './'

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

gerritbot added a project: Patch-For-Review.Mar 24 2025, 8:31 PM

• Hokwelum moved this task from Not planned (Patches welcome!) to In progress (DO NOT USE) on the MediaWiki-Platform-Team board.Mar 24 2025, 8:32 PM

Change #1130717 merged by jenkins-bot:

[mediawiki/core@master] ResourceLoader: Improve error when including file in non-package module

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

Maintenance_bot removed a project: Patch-For-Review.Mar 26 2025, 6:30 AM

ReleaseTaggerBot added a project: MW-1.44-notes (1.44.0-wmf.23; 2025-04-01).Mar 26 2025, 7:01 AM

• Hokwelum closed this task as Resolved.Mar 26 2025, 9:25 PM

Change #1131468 had a related patch set uploaded (by Krinkle; author: Hokwelum):

[mediawiki/core@master] ResourceLoader: Reject module names starting with "./" && "../"

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

gerritbot added a project: Patch-For-Review.Mar 27 2025, 8:13 PM

Change #1131468 merged by jenkins-bot: