fix: 🐛 fix the issue 17689, cache process env value on startup to…

[gnekich]

… allow changing process.env in vite.config.js without impact to loadEnv function

Description

fixes #17689 , this fix caches process.env values on vite server startup in order to enable developers to change process.env in vite.config.js using loadEnv while not losing the ability to hot reload env value changes in dot env files without the need for a server restart.

This PR enables developers to do this;

import { defineConfig, loadEnv } from "vite";
import react from "@vitejs/plugin-react";

// https://vitejs.dev/config/
export default ({ mode }) => {
  process.env = { ...process.env, ...loadEnv(mode, process.cwd()) };

  console.log("process.env.VITE_CHANGE_ME", process.env.VITE_CHANGE_ME);

  return defineConfig({
    plugins: [react()],
  });
};

also making the currently most voted answer on StackOverflow working correctly and improving DX.

[stackblitz]

Run & review this pull request in StackBlitz Codeflow.

[bluwy]

I'm not sure if this is very safe and it makes the function less pure. I'd lean towards keeping it as is so the function will always operate on the current process.env value.

At any point in time, the current process.env.VITE_SOMETHING value will always take precedence instead of from an arbitrary first call that some libraries may not have control of.

[gnekich]

I'm not sure if this is very safe and it makes the function less pure. I'd lean towards keeping it as is so the function will always operate on the current process.env value.

At any point in time, the current process.env.VITE_SOMETHING value will always take precedence instead of from an arbitrary first call that some libraries may not have control of.

Hi @bluwy , first of all thanks for taking the look into this.

It seems to me completely safe, do you see how this could not be the case?
I agree with you that it is not pure as it saves the cached value to global state, but that function is already not pure as it is explicitly modifying/setting external global process.env.VITE_USER_NODE_ENV, process.env.BROWSER, process.env.BROWSER_ARGS, etc.

What would be your suggestion @bluwy ? I would also like that function operates on process.env but server currently wont reload existing values as demonstrated in PoC. We can detach this from first run of the function and put it on vite server startup, maybe. 🤷🏻 But there is also nothing wrong in doing the check in the loadEnv if cache exists.
We could add more optional args to loadEnv, or create new loadEnvCached function.

Ironically the function is exactly the same level of "pureness" now as it will return the same output for the same args in all cases, before and after the fix. (*.env files are loaded inside of the functions outside of args., process.env is modified and reed outside of args.)

If a library changes process.env.VITE_SOMETHING="1337" it takes precedence, if you pass it inline it takes precedence, the output/return value will contain VITE_SOMETHING="1337".

So I would argue that there is no issue, I may be wrong, if someone could make quick PoC or explain where this fails I would be grateful.

Here to help,
Best Regards

[bluwy]

The function isn't pure which is true, but the PR is tainting that more.

Previously: Given the same process.env conditions and same .env values and the time loadEnv() is called, you can be guaranteed that it'll always return the same value. From the individual consumer point of view, this is consistent.

Now: The process.env value is determined during the first call of loadEnv(). So given the same .env values, depending on the order of loadEnv() is called, you could be getting different results.

This makes it harder to debug, e.g. now I need to care when loadEnv() is first called by someone else, which I may not have control of. Many frameworks use loadEnv() for various reasons, and this will make any issues with env handling even unpredictable.

The current implementation isn't perfect, but it's more predictable and consistent.