fix: don't store temporary vite config file in node_modules if deno

[kazushisan]

Description

fixes #18822

This fixes an issue where Vite v6 does not work on Deno when import statements with the specifier npm: is used in the config file.

Vite will create a bundled version of vite.config.ts under node_modules/.vite-temp if node_modules/ exits. Import statements like the following will be kept in the generated output.

import { defineConfig } from 'npm:vite@6'

When this generated file is import() ed by Vite running on Deno, Deno will throw an error TypeError: [ERR_UNSUPPORTED_ESM_URL_SCHEME]. This is because Deno assumes that modules under node_modules/ is a valid file for executing with Node.js and considers the npm: prefix invalid.

This patch will check if the the environment is Deno and will prevent from storing the temp file under node_modules/ if its Deno.

[kazushisan]

I think that ideally this condition should be checking if the env is Node.js or not but there doesn't seem to be a way to reliably detect that

[sapphi-red]

I think this is special behavior on the Deno side and correct to branch on Deno.

[kazushisan]

In my opinion, assuming that arbitrary files (not npm packages) placed in node_modules/ will work regardless of the runtime is quite a leap compared to storing files meant to be consumed in the browser in node_modules/.vite.

I believe that placing application files under node_modules/ and executing them with a runtime is not something that a runtime (or package manager) would anticipate. Therefore, I would argue that the current design relies on the specific behavior of Node.js and should not be assumed to work universally across all runtimes.

(I understand it's not my call and it's definitely out of scope but just wanted to share my thoughts 💭)

[sapphi-red]

I think it is just a matter of perspective. The reason Vite works with deno is because deno is fairly compatible enough with Node, not because Vite tries to support all existing runtimes (not saying deno isn't supported, it's just how it currently works). From that point of view, deno lacks Node.js compatibility here, and Vite applies a patch for deno (that said, I personally think that behaving differently in node_modules is a valid choice on deno's side).

[kazushisan]

Totally agree, it is a matter of perspective but I was just curious if this fairly uncommon approach (even from node's standards) to store modules that are executed by the host runtime in node_modules/ is worth the benefits compared to the potential breaks that it may introduce for other runtimes.

Anyway, thanks for elaborating 👍

[sapphi-red]

It's unfortunate that we cannot use node_modules to write the temporary file. I don't come up with other ways. I think we should go with this one.

[sapphi-red]

I think this is special behavior on the Deno side and correct to branch on Deno.

[sapphi-red]

const hasNpmPrefixExternalImport = Object.values(result.metafile.outputs)
    .flatMap((output) => output.imports)
    .filter((imp) => imp.external && imp.path.startsWith('npm:'))

Does passing this value from bundleConfigFile and only avoid writing to node_modules in that case work? If I understand correctly, the problem only happens if the config file has an import with npm: prefix.

If you have a code like below, it won't work. But this is already not supported, so it should be fine.

const dynImport = (str: string) => import(str)

dynImport('npm:foo')

[kazushisan]

If I understand correctly, the problem only happens if the config file has an import with npm: prefix.

Yes, you're right! Here's a break down of what's going to happen for clarity.

1. Vite removes the npm: prefix from import statements
2. bundled file is executed with Deno
3. Deno's module resolution expects an npm: prefix when we want to do something like import { defineConfig } from 'vite';
4. However since this bundled file is placed in node_modules/, Deno will consider the file as an Node.js compatible module and resolve import statements accordingly
5. import works!

So your solution should work, but I think it make thing too unpredictable and fragile because it's like trying to implement a transformation layer that voids Deno's behavior difference for modules inside/outside of node_modules/

Since the bundled file is going to be executed by Deno not Node, I'd say that keeping the import statements as-is is a better solution.

[kazushisan]

@sapphi-red thanks for the prompt review! I've addressed your comments!

[sapphi-red]

Thank you!