relative url resolving in JS via toOutputFilePathInString may introduce the need for the module param in SystemJS

[Tal500]

Describe the bug

Background

While working on SvelteKit legacy feature (sveltejs/kit#12), I had encountered an issue on Svelte CSS resolving of relative URL parameters with the Vite legacy plugin.
After a lot of digging, it turned out that the problem I had wasn't related to Svelte, nor to Vite legacy plugin nor to even Babel - but to the internal Vite relative URL resolving in JS output code, when rendering SystemJS chunks.
I have gave this unrelated background so you folks will hopefully forgive me I'm not giving a complicated reproduction example(with too many forks), but rather prove to you by the code structure and notes what and why the problem exists.
I have modified Vite source code to fix the problem I'll describe next, and then my issues with SvelteKit legacy build with the relative URL were also fixed. Will send PR soon.

Problem

The function toOutputFilePathInString takes a path, and returns a JS code representation that computes the path (the computation is needed if the path is relative).
Following the default parameter, you'll see it uses internally relativeUrlMechanisms, and notice the following suspicious line:

vite/packages/vite/src/node/build.ts

Lines 887 to 888 in b2c0ee0

	// NOTE: make sure rollup generate `module` params
	system: (relativePath) => getResolveUrl(`'${relativePath}', module.meta.url`),

Turns out that this code comment wasn't handled at all. It means is that:

Failing Case Story

When rendering chunks in SystemJS format, you might start with a code that looks like:

System.register([], function () {
    'use strict';
    // ...
}

but you may ends up using (internally) relativeUrlMechanisms, which may use the definition of the missing module parameter.
That means, the new code should include the module parameter and be in the form of:

System.register([], function (exports, module) {
    'use strict';
    // ...
}

Well, this "edge-case" was common in my use-case (introduced above).
This mean that I get in the legacy browser an error of "module" is not defined, since Vite internal code uses toOutputFilePathInString to transform relative URLs to JS code, which resolved to be in the following shape, which uses the missing parameter 'module':

vite/packages/vite/src/node/build.ts

Line 888 in b2c0ee0

system: (relativePath) => getResolveUrl(`'${relativePath}', module.meta.url`),

Solution

Need to introduce a property in toOutputFilePathInString, that tells whether the definition for the "module" is needed for the JS expression.
We need to propagate this property all over until we return to the rendering chunk function (in the internal plugins that use input paths in code). Then, on the final rendering step we need to make sure that the final code have the "module" definition (if not, replace the function deceleration to be in the signature function (exports, module)).

As said above, I'll post a PR that do exactly this.

Reproduction

no need to, I proved the problem by the code structure

System Info

N/A

Used Package Manager

pnpm

Logs

No response

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to vuejs/core instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.