Skip to content

Load .js user scripts through a file URL so they work on Windows - #2570

Open
FurkanKra wants to merge 1 commit into
sitespeedio:mainfrom
FurkanKra:fix/windows-user-script-loading
Open

FurkanKra wants to merge 1 commit into
sitespeedio:mainfrom
FurkanKra:fix/windows-user-script-loading

Conversation

@FurkanKra

Copy link
Copy Markdown

Loading a .js (or .ts) user script fails on Windows:

ERROR: Could not parse user script C:\tmp\btscript\myscript.js with error
Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only URLs with a scheme in: file, data,
and node are supported by the default ESM loader. On Windows, absolute paths
must be valid file:// URLs. Received protocol 'c:'

loadFile passes an absolute path straight to import():

const myFunction = await import(path.resolve(script));

On Windows an absolute path has to be a file:// URL. The branch 24 lines above
— for .mjs/.cjs/.mts/.cts and --cjs false — already does this
correctly with pathToFileURL, so only the default .js/.ts branch is
affected.

Impact

It depends on whether the caller passes throwError:

  • loadPrePostScripts does, so --preScript / --postScript fail with a
    message that blames the user's script rather than the path conversion.
  • Callers that don't get undefined back, and the script is silently skipped.

Windows CI didn't catch it because every script under test/data is .cjs,
which takes the branch that already converts correctly.

Second change: clean up the temporary package.json on failure

The unlink of the generated package.json sat after the await import(...)
inside the same try, so any load failure skipped it and left the file behind
in the user's script directory. I moved it to a finally.

This one is not Windows specific — a user script with a syntax error leaves the
file behind on every platform. Happy to split it into its own PR if you'd
rather keep them separate.

Verification

Added test/unittests/engineUtilsTest.js with two tests. They split cleanly
along the two problems:

Linux Windows
Load a .js user script passed before (absolute paths are fine on POSIX) failed before, passes after
Remove the temporary package.json failed before, passes after failed before, passes after
  • Windows 11, Node.js 25.9: reproduced both failures against main, both pass
    with the change. Also confirmed the stray package.json was left in the
    script directory before, and is gone after.
  • Linux, on my fork with a temporary workflow running just this test file:
    without the fix it fails,
    with the fix it passes.
  • npx eslint and npx tsc are clean.

One more small effect: when a user script genuinely has a syntax error, Windows
now reports that SyntaxError instead of masking it behind
ERR_UNSUPPORTED_ESM_URL_SCHEME.

Not covered: I could not run the full npm test suite locally — it drives
real browsers, and the driver postinstall doesn't complete on my machine. I ran
lint, tsc and the new test file only, plus the Linux runs linked above. Worth a
full CI run before merging.

If it's useful, adding a step to windows.yml that runs a .js script (the
existing ones are all .cjs) would stop this from coming back — I left that out
to keep the diff small, but happy to add it.


AI disclosure, per CONTRIBUTING: this change was made with assistance from
Claude. I reviewed every line, reproduced both failures on Windows myself
before and after, and set up the two Linux runs above specifically to check
that the new test really is red without the fix rather than assuming it.

loadFile passed an absolute path straight to import() when loading a
.js or .ts user script. On Windows that throws
ERR_UNSUPPORTED_ESM_URL_SCHEME, because absolute paths there have to be
file:// URLs. The branch a few lines above, for .mjs/.cjs/.mts/.cts and
--cjs false, already converts the path with pathToFileURL, so only the
default branch was affected.

The effect on Windows depends on the caller. loadPrePostScripts passes
throwError, so --preScript and --postScript fail with a message that
points at the user's script rather than at the path conversion. Callers
that do not pass throwError get undefined back and the script is
silently ignored.

Also move the cleanup of the temporary package.json into a finally
block. It ran after the await, so any failure to load the script left
the generated package.json behind in the user's script directory. That
part is not Windows specific: a user script with a syntax error leaves
the file behind on every platform.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant