Preload modules from `require` calls #5319

mstoykov · 2025-10-17T09:48:57Z

What?

Now when a loaded js module has a require function call its arguTo be disabment (if string) will be loaded as a module and we will repeat the process.

This should load all modules, and more importantly will help with finding if any missing modules can not be loaded.

This is very likely not a good idea in general, but is what esbuild is doing currently if auto extension resolution is enabled. So this is copying this behaviour.

It is likely that this will be disabled in the future (v2) and might need or not need additional testing.

Why?

This is what currently happens in esbuild+k6deps dependancy analysis so this just copies it

Checklist

I have performed a self-review of my code.
I have commented on my code, particularly in hard-to-understand areas.
I have added tests for my changes.
I have run linter and tests locally (make check) and all pass.

Checklist: Documentation (only for k6 maintainers and if relevant)

Please do not merge this PR until the following items are filled out.

I have added the correct milestone and labels to the PR.
I have updated the release notes: link
I have updated or added an issue to the k6-documentation: grafana/k6-docs#NUMBER if applicable
I have updated or added an issue to the TypeScript definitions: grafana/k6-DefinitelyTyped#NUMBER if applicable

Related PR(s)/Issue(s)

#5320 combines this and #5240 to reimplement auto extension resolution without esbuild and k6deps.
Issue/discussion for stopping this in v2 #5325

Now when a loaded js module has a `require` function call its argument (if string) will be loaded as a module and we will repeat the process. This should load all modules, and more importantly will help with finding if any missing modules can not be loaded. This is very likely not a good idea in general, but is what esbuild is doing currently if auto extension resolution is enabled. So this is copying this behaviour. It is likely that this will be disabled in the future (v2) and might need or not need additional testing.

mstoykov · 2025-10-21T09:21:42Z

js/modules/resolution.go

+	// TODO(@mstoykov): put this behind a flag
+	if err != nil {


This seems not as easy as I was hoping, and I would like to first confirm that we will want to have a flag to disable this, And also if we agree to disable this in v2, as well.

Do we guarantee it somewhere? If we haven't provided any guarantee for it then why we do not stop supporting it right now?

cc @pablochacin @szkiba

I am not against it. BUt we do not currently have any data on how used it is. And I am not certain we have no place where we give this as an example or something.

@mstoykov If I understand it correctly, you mean the pre-loading or required modules during the dependency analysis, right? Also, I understand we support require at execution time. So, if we stop analyzing the required modules, wouldn't it create inconsistent behavior by not recognizing the dependencies from those modules?

require is runtime/exucution time constract.

This code similar to what esbuild does, is going through the code and looking at anything that seems to be a require call and if the arguments is a string literal - great, we preload it.

This (as mentione in #5325) isn't ... very reliable to begin wiht. For simple cases it definitely does work, but it both doesn't work for cases where the argument is a variable (actaully not uncommon for complex scripts IME) and it also definitely over finds stuff IME both now and in esbuild implementation. The esbuild technically does better, by dropping "dead" code ... but even that seems to be very uncosistant in me experimentation. Where for some simple cases it works and for a slightly different one it apperantly can not drop the code.

Also obviously you can put require behind a if statement in which case it is even more of ... should this be imported at all.

But all that aside - require and commonJS are not standard and hopefully ... they will not be used a lot.

So IMO we should not have this hacky ways to have and instead should ask people to use ESM.

This btw is also going to have to be true for dynamic imports when we support them ... and I do not think doing that is good idea too.

mstoykov · 2025-10-21T09:23:15Z

I have moved this to not be dependent on #5240, but this PR is still pretty much a dependency for #5320 even if code wise it isn't a problem. As otherwise we will stop preloading require calls and this might break some scripts.

codebien

The current version LGTM, but if we don't want to support it then I think we should drop it now instead of waiting for v2.

js/modules/resolution.go

js/modules/cjsmodule.go

codebien · 2025-10-21T12:45:15Z

js/modules/resolution.go

 	}
+	// TODO(@mstoykov): put this behind a flag
+	if err != nil {
+		potentialRequireCalls := findRequireFunctionInAST(prg.Body)


Suggested change

potentialRequireCalls := findRequireFunctionInAST(prg.Body)

requireCalls := findRequireFunctionInAST(prg.Body)

Why do we call them potential? I guess they are require calls or not. What did I miss?

Here potential means htat ... they might not actually get called - they are part of the code, but this doesn't analyse what is called, just what code exists.

js/modules/resolution.go

codebien · 2025-10-21T12:51:13Z

js/modules/resolution.go

+	// TODO(@mstoykov): put this behind a flag
+	if err != nil {


Do we guarantee it somewhere? If we haven't provided any guarantee for it then why we do not stop supporting it right now?

codebien · 2025-10-21T12:53:09Z

js/modules/cjsmodule.go

+	return result
+}
+
+func findRequireFunctionInStatement(i ast.Statement) []string { //nolint:cyclop,funlen


Suggested change

func findRequireFunctionInStatement(i ast.Statement) []string { //nolint:cyclop,funlen

// findRequireFunctionInStatement finds ...

func findRequireFunctionInStatement(i ast.Statement) []string { //nolint:cyclop,funlen

js/modules/cjsmodule.go

Co-authored-by: Ivan <2103732+codebien@users.noreply.github.com>

Reimplement auto extension resolution without using esbuild or k6deps, but the internal methods that k6 uses to load modules. This removes the need for a separate project to have the same way to parse and understand imports and also double parsing everything. Also fixing a number of bugs coming from this and removes the maintenance burden to keep both projects align. It also fixes problems with paths on windows as it is now using the internal handling everywhere. Makes use directives actually only work at the beginning of files, but also for all files. It does now not work with `require` calls, at least until/if #5319 is merged. Correctly propagates exit codes from sub processes up. Fixes #5127 FIxes #5176 Fixes #5104 Closes #5102 Likely some updates to every other issue around automatic extension resolution as well. Co-authored-by: Ivan <2103732+codebien@users.noreply.github.com> Co-authored-by: Théo Crevon <oleiade@users.noreply.github.com>

mstoykov added this to the v1.4.0 milestone Oct 17, 2025

mstoykov temporarily deployed to azure-trusted-signing October 17, 2025 09:54 — with GitHub Actions Inactive

mstoykov temporarily deployed to azure-trusted-signing October 17, 2025 09:56 — with GitHub Actions Inactive

mstoykov force-pushed the requirePreloading branch from 9fdb1fd to 9d8a30f Compare October 17, 2025 15:34

mstoykov force-pushed the allUnknownModulesDetection branch from 4d232bb to 2142f1a Compare October 17, 2025 15:37

mstoykov force-pushed the requirePreloading branch from 9d8a30f to 2d2a426 Compare October 17, 2025 15:37

mstoykov temporarily deployed to azure-trusted-signing October 17, 2025 15:44 — with GitHub Actions Inactive

mstoykov temporarily deployed to azure-trusted-signing October 17, 2025 15:48 — with GitHub Actions Inactive

mstoykov mentioned this pull request Oct 20, 2025

Stop preloading require calls and using them to do Auto Extension Resoluton #5325

Open

mstoykov force-pushed the requirePreloading branch from 2d2a426 to b7254c6 Compare October 21, 2025 09:14

mstoykov added 2 commits October 21, 2025 12:17

add usage/requirePreload reporting

4d2e6d2

mstoykov marked this pull request as ready for review October 21, 2025 09:19

mstoykov requested a review from a team as a code owner October 21, 2025 09:19

mstoykov requested review from codebien and oleiade and removed request for a team October 21, 2025 09:19

mstoykov changed the base branch from allUnknownModulesDetection to master October 21, 2025 09:20

mstoykov force-pushed the requirePreloading branch from b7254c6 to 4d2e6d2 Compare October 21, 2025 09:20

mstoykov commented Oct 21, 2025

View reviewed changes

mstoykov temporarily deployed to azure-trusted-signing October 21, 2025 09:26 — with GitHub Actions Inactive

mstoykov temporarily deployed to azure-trusted-signing October 21, 2025 09:28 — with GitHub Actions Inactive

mstoykov temporarily deployed to azure-trusted-signing October 21, 2025 09:29 — with GitHub Actions Inactive

mstoykov mentioned this pull request Oct 21, 2025

Refactor auto extension resolution to not use k6deps and esbuild #5320

Merged

8 tasks

codebien reviewed Oct 21, 2025

View reviewed changes

mstoykov and others added 2 commits October 23, 2025 11:06

Apply suggestions from code review

e67feb7

Co-authored-by: Ivan <2103732+codebien@users.noreply.github.com>

fixup! Apply suggestions from code review

6a5d983

mstoykov temporarily deployed to azure-trusted-signing October 23, 2025 08:23 — with GitHub Actions Inactive

mstoykov temporarily deployed to azure-trusted-signing October 23, 2025 08:25 — with GitHub Actions Inactive

mstoykov requested review from codebien and pablochacin October 27, 2025 09:19

mstoykov added 2 commits October 27, 2025 19:52

Run the require preloading if there are already no errors

b931037

Add K6_EXPERIMENTAL_REQUIRE_PRELOADING flag defaulting to false

632e168

mstoykov removed this from the v1.4.0 milestone Nov 3, 2025

codebien removed their request for review November 12, 2025 10:46

mstoykov added area: Auto Extension Resolution Previously known as binary provisioning. evaluation needed proposal needs to be validated or tested before fully implementing it in k6 labels Nov 18, 2025

	potentialRequireCalls := findRequireFunctionInAST(prg.Body)
	requireCalls := findRequireFunctionInAST(prg.Body)

	func findRequireFunctionInStatement(i ast.Statement) []string { //nolint:cyclop,funlen
	// findRequireFunctionInStatement finds ...
	func findRequireFunctionInStatement(i ast.Statement) []string { //nolint:cyclop,funlen

Preload modules from require calls #5319

Are you sure you want to change the base?

Preload modules from require calls #5319

Conversation

mstoykov commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What?

Why?

Checklist

Checklist: Documentation (only for k6 maintainers and if relevant)

Related PR(s)/Issue(s)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mstoykov commented Oct 21, 2025

Uh oh!

codebien left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Preload modules from `require` calls #5319

Preload modules from `require` calls #5319

mstoykov commented Oct 17, 2025 •

edited

Loading