fix: use environment config root for hmr filename

[lazarv]

Description

When using multiple environments with different root in the configuration, HMR emits incorrect file paths as shortName is resolved using the main config instead of the environment's config.

This PR fixes the issue by not using the resolved short filename for the updateModules call and using the environment config when working against a specific environment.

No tests failed, Vite dev server works as expected, but HMR payload is incorrect as it contains a non-existing triggeredBy path.

Fixes #17393

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[lazarv]

Hi @patak-dev @antfu @sheremet-va, maybe one of you will know what's happening here! With my changes the tests for the hmr should not reload if no accepted within circular imported files are failing unpredictably, for each run it fails in a different test environment, while working locally all the time. I suspect there's some async issue here, but I don't had the time yet to figure out where, in Vite or in tests. I will get back to this as I need this fix, but all of you know Vite much better than me. Thanks for the help!

[patak-dev]

The test are also flaky on the v6 branch. Don't worry about that. And thanks for experimenting and giving feedback about Environment API.
About the changes, environment.config is exactly the same as the shared config. So root from there will still be shared for all environments. We're thinking on renaming to environment.sharedConfig to avoid this confusion. The one you are looking for is environment.options.

About making root per-environment, I'm not sure we want to go there. root is quite fundamental. I think we should have a very good use case to justify this. Would you expand on why you need to modify this value per environment?

[lazarv]

Hi, thanks for your answer @patak-dev!

environment.config is exactly the same as the shared config

In my experimentation, I created my environments using createEnvironment and instantiated new envs, overriding the config object for some props, like root. This works as I expected, module resolution uses this new root for this environment. My only issue was that during HMR, the triggeredBy path is incorrect, see more details and a repro in the linked issue at #17393.

ssr: {
  dev: {
    createEnvironment: (name, config) => {
      return createNodeDevEnvironment(
        name,
        {
          ...config,
          root: rootDir,
          // ...

But yes, you're right, normally, when only specifying some overrides without creating an environment directly like above, the config will be the same, as it's just passed over at https://github.com/vitejs/vite/blob/v6/environment-api/packages/vite/src/node/server/index.ts#L489.

I make all of this hussle around Vite configurations, module loaders and even using module-alias package to always use the react/react-dom/react-server-dom-webpack version provided by the my framework, as new React features are not working with any version of React. This will be much simpler, when React 19 finally be released. Convincing all parties to use React from the framework requires a lot. Maybe I'll drop this approach, but I would like to be able to only install the framework and no React dependencies directly.

If you would like to have some more direct example, I can try to move my Vite Env API experiment into StackBlitz.

[patak-dev]

I still don't see why you need a different root for each environment from the explanation above. Would you expand on this more? Or are you saying it is ok for you to have a single root for the project (this is what I think we should enforce). There is a limit in how much flexibility we can give to configuring environments. We reach a point where what you should be doing is creating several Vite Servers.

[lazarv]

No, right now I need different root for environments, but I'm saying that this is very much possible right now and it's working as I expect it to work if I create new environments like in the code in my previous comment. I used 3 Vite servers before using the Env API for the client, RSC and SSR environments. I don't need any new features added to Vite, only the HMR fix to get the proper triggeredBy path which is incorrect right now as current Vite implementation is based on that the config object reference is the same, but when using createEnvironment and patching the config object, it will be not the same. I think with the Env API, it's very much likely possible to create Vite "servers" on their own, without any HTTP/WS layers added, for me it's basically that solution and perfect for my use case.

• client environment is responsible for serving client components to the browser, needs default conditions to resolve React, basically this is the only environment where I use the root as the app's current working directory, this is running on the main thread, root is cwd
• rsc environment is where the framework renders RSC payload, needs react-server conditions to resolve React, but from inside the framework to use a specific version of React, this also runs on the main thread, but root is the frameworks's directory
• ssr environment is to render HTML in a worker thread, using the RSC payload stream, needs default conditions to resolve React, but also needs to use the specific React version provided by the framework, root is the framework's directory

I spent way too much time to simplify the setup, but for what I wanted to achieve, this was the only way it worked, with multiple roots, if not, then React module resolution was not working properly.

You can get a closer look at the Vite server setup at https://github.com/lazarv/react-server/blob/vite6-environments-api/packages/react-server/lib/dev/create-server.mjs#L70-L294.

[patak-dev]

Check out https://github.com/hi-ogawa/vite-environment-examples, there are examples of doing RSC without multiple roots there. It is ok if you'd like to keep separate servers. For Environment API, I think we need to be careful in what we make per-environment. It seems it is possible to achieve what you want without separate roots, so for now, let's close this one.

[lazarv]

Sorry to hear! My use case is more advanced than that example, I already checked it out before. It is not possible what I want to achieve without using multiple roots. But I was very satisfied with the Environment API, so I'll resolve this issue on my side and use it as-is. Thanks for discussing this!