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

mstoykov · 2025-10-17T15:53:33Z

What?

Reimplement auto extension resolution without using esbuild or k6deps, but the internal methods that k6 uses to load modules.

Why?

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 maintaince burden to keep both projects align.

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)

Based on #5240 and requires #5319 to have the same functionality as the same.

Fixes #5127
FIxes #5176
Fixes #5104
Closes #5102
Requirement for #5166 ( likely a future PR that refactors more of test_load.go)

Likely some updates to every other issue around automatic extension resolution as well.

mstoykov · 2025-10-21T10:49:29Z

This likely should get more tests, and either drop the launcher.go and the old way or to further make it possible to use the old resolution method ... for now at least.

But I would like some initial feedback and thoughts.

codebien · 2025-10-21T13:08:57Z

internal/cmd/launcher.go

+			constraints := string(match[idxUseConstraints])
+
+			if _, ok := deps[extension]; ok {
+				return deps, fmt.Errorf("already had a use directivce for %q", extension)


Suggested change

return deps, fmt.Errorf("already had a use directivce for %q", extension)

return deps, fmt.Errorf("already had a use directive for %q", extension)

Is this printed to the user? Can we make it a bit more user friendly? I guess if this appears out of nowhere the user is not getting it fast.

Please see it now.

internal/cmd/root.go

internal/cmd/test_load.go

codebien · 2025-10-21T13:44:58Z

internal/cmd/test_load.go

+// TODO(@mstoykov) potentially figure out some less "exceptionl workflow" solution
+type runDifferentBinaryError struct {
+	customBinary commandExecutor
+}


To be honest, that approach seems it introduces not very linear logic which is difficult to follow and remember. I'd appreciate if we try to find a better way before merge it.

I do agree with you which is why I left the comment.

Unfortunately having gone though doing the changes ... this seems like the most concise and not convuleted way to do it.

Effectively we want to stop executing the current workflow and run a new program.

Currently we use the fact that returning an error will do that, and we return errors from the different places where the source is loaded in the same way. They all propagate it backwards and then in the root command where nothing else will happen otherwise we check if it is the specific error and run the command, instead of errroring.

In all other cases all this backtracking will need to be codified, which likely will mean a lot more changes to signatures of functions and changes to run, cloud( and cloud run), archive, inspect and ... propagate it backwards? Have it run here and return backwards to stop?

The only other not very involved way I can see is to execute here , and return a "do nothing error", which seems worse as it is now an error you should propagate backwards but the root still needs to figure out it should not do anything with it.

@mstoykov Can you open an issue and assign to the next milestone so we don't lose track of it, please?

pablochacin

I just gave a first pass and centered my comments on the structure of the code.

I find it unintuitive that we use an error for actually handling the missing dependencies. I would prefer if we could split the logic and use the error for reporting and let someone else (the root command?) to handle it by attempting binary provisioning.

pablochacin · 2025-10-22T15:30:33Z

internal/cmd/root.go

+	var differentBinaryError runDifferentBinaryError
+
+	if errors.As(err, &differentBinaryError) {
+		err = differentBinaryError.customBinary.run(c.globalState)


I find getting the custom binary from the error unintuitive.
I would expect that the error was about missing dependencies and handle it here by attempting the binary provisioning. That would have a better separation of concerns between the error and how it is handled.

I do agree on that part, but as I explain in https://raintank-corp.slack.com/archives/C07CJ4T7KN1/p1761206377837119?thread_ts=1761122691.317629&cid=C07CJ4T7KN1 , the alternative is probably twice as many changes if not more. and likely still very strange behaviours.

This way we unroll back to a state where I know nothing is happening and then running the new binary, if possible.

Please move the discussion there. And if this is the problem, I will do the changes, but given the time restrictions on v1.4.0 approaching, the complexity and the importantce of the changes, I would prefer to not keep adding stuff. I already had quite a lot of problems with stuff that ... I would define as implementation quirks

I don't follow you. Unless I'm mistaken, the Slack thread you are pointing me to is about another unrelated topic (how we should resolve pragmas).

My comment is about the structure of the code. Regardless of how we find out the missing extensions, I think we should separate that from obtaining the binary.

And I think that discussion should happen here as it is related to the code.

Aaah ,... sorry.

Do you mean like e48314e ?

internal/cmd/root.go

internal/cmd/test_load.go

mstoykov · 2025-10-24T10:37:59Z

I think I have gone through all the points and I also did some refactoring to make it easier to follow.
I also feel like now we probably do not need more tests. Does anyone has any other ideas?

cc @codebien @pablochacin

codebien

@mstoykov much better now, thanks for the effort 🙇

LGTM, as soon as we have a green CI.

internal/cmd/root.go

pablochacin

Thanks for the changes. I like this structure more. I left some comments, none of them blocking.

internal/cmd/root.go

pablochacin · 2025-10-24T13:18:02Z

internal/cmd/launcher.go

+	return nil
+}
+
+func findDirectives(text []byte) []string {


What's the reason for not using regexp from k6deps for this analysis? wouldn't it be more compact?

I needed to fix them which turned out to not work particularly well

I ... did find they are slightly buggy in strange cases such as "use k6 with k6/x/sql with something" is ... .okay and it parsed it as "use k6 something" if I remember correctly

I do not think it is more readable and I am pretty sure it is 1-2 lines shorter.

3cca41f you can see that all the definitions are basically more than half the current implementation.

And I do not think I am the only person who thinks the regex are ... not exactly the easiest thing to debug.

pablochacin

@mstoykov I didn't mean to ask for changes (I'm pretty sure I only submitted comments), so please ignore that. Github and its mysterious ways....

Co-authored-by: Ivan <[email protected]>

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

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

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

mstoykov mentioned this pull request Oct 21, 2025

Preload modules from require calls #5319

Open

8 tasks

mstoykov force-pushed the basicNativeNativeBinaryProvisioning branch from 5c93d1d to d738041 Compare October 21, 2025 10:34

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

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

mstoykov marked this pull request as ready for review October 21, 2025 10:47

mstoykov requested a review from a team as a code owner October 21, 2025 10:47

mstoykov requested review from ankur22, codebien, oleiade and pablochacin and removed request for a team October 21, 2025 10:47

mstoykov mentioned this pull request Oct 21, 2025

Fix auto extension resolution breaking tests on windows #5206

Closed

8 tasks

mstoykov changed the title ~~Thread the running the binary up enough~~ Refactor auto extension resolution to not use k6deps and esbuild Oct 21, 2025

codebien reviewed Oct 21, 2025

View reviewed changes

Base automatically changed from allUnknownModulesDetection to master October 21, 2025 14:17

mstoykov force-pushed the basicNativeNativeBinaryProvisioning branch from 65afd95 to c8e52fa Compare October 22, 2025 08:56

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

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

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

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

pablochacin requested changes Oct 22, 2025

View reviewed changes

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

mstoykov temporarily deployed to azure-trusted-signing October 24, 2025 10:41 — with GitHub Actions Inactive

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

codebien previously approved these changes Oct 24, 2025

View reviewed changes

internal/cmd/root.go Outdated Show resolved Hide resolved

mstoykov dismissed codebien’s stale review via 7fd1132 October 24, 2025 11:00

mstoykov temporarily deployed to azure-trusted-signing October 24, 2025 11:06 — with GitHub Actions Inactive

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

pablochacin reviewed Oct 24, 2025

View reviewed changes

mstoykov requested a review from pablochacin October 24, 2025 13:29

mstoykov and others added 16 commits October 24, 2025 18:52

Thread the running the binary up enough

01395e8

Add test from k6deps

1055219

Add tests around paths especially on Windows

3d49157

rename

da53b3c

Update internal/cmd/root.go

4d5ca1e

Co-authored-by: Ivan <[email protected]>

panic on Error() that should never be called

c239938

move whole binary provisioning in the root command

4fb9c14

Drop fmt.Println

98b7fe7

Parse use directives just at the start

9cb8329

Move away from sobek to find directives

3fb6d10

Add dependancy type and fix errors and logs around loading files

a01ddac

do not set the launcher to nil

f024dad

Fix some corner case panics

49d1ae9

Fix #5327 for K6_OLD_RESOLUTION as well

9df44eb

Update internal/cmd/root.go

25378d3

Co-authored-by: Ivan <[email protected]>

propagate exit codes from subprocess

ca4a183

mstoykov force-pushed the basicNativeNativeBinaryProvisioning branch from 38432f1 to ca4a183 Compare October 24, 2025 15:54

mstoykov temporarily deployed to azure-trusted-signing October 24, 2025 16:01 — with GitHub Actions Inactive

mstoykov temporarily deployed to azure-trusted-signing October 24, 2025 16:02 — with GitHub Actions Inactive

	return deps, fmt.Errorf("already had a use directivce for %q", extension)
	return deps, fmt.Errorf("already had a use directive for %q", extension)

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

Are you sure you want to change the base?

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

Uh oh!

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!

mstoykov commented Oct 21, 2025

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!

Uh oh!

Uh oh!

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!

pablochacin left a comment

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mstoykov commented Oct 24, 2025

Uh oh!

codebien left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pablochacin 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!

Choose a reason for hiding this comment

Uh oh!

pablochacin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mstoykov commented Oct 17, 2025 •

edited

Loading

pablochacin left a comment •

edited

Loading