Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HMR of env variable is not working #28891

Open
XantreDev opened this issue May 15, 2024 · 5 comments
Open

HMR of env variable is not working #28891

XantreDev opened this issue May 15, 2024 · 5 comments
Labels
CLI Versioned Expo CLI -- `npx expo start` Issue accepted

Comments

@XantreDev
Copy link

XantreDev commented May 15, 2024

Summary

Changing of .env file do not change anything

Problems lays in env expansion code:

  expo:start:server:metro:waitForTypescript Observed change: C:\Users\Xantre\Projects\fleet-mobile\apps\international\.env +2s
  expo:start:server:metro Reloading environment variables... +2s
  expo:env Loaded environment variables from: C:\Users\Xantre\Projects\fleet-mobile\apps\international\.env +2s
before expand {
  EXPO_PUBLIC_SHOW_DEBUG_SCREEN: 'beb',
}
updated env {
  env: {
    EXPO_PUBLIC_SHOW_DEBUG_SCREEN: 'sdfsdf',
}

What platform(s) does this occur on?

Android, iOS

SDK Version

~50.0.17

Environment

expo-env-info 1.2.0 environment info:
System:
OS: Windows 11 10.0.22631
Binaries:
Node: 20.12.0 - ~\scoop\apps\volta\current\appdata\tools\image\node\20.12.0\node.EXE
Yarn: 1.22.21 - ~\scoop\apps\volta\current\appdata\tools\image\yarn\1.22.21\bin\yarn.CMD
npm: 10.5.0 - ~\scoop\apps\volta\current\appdata\tools\image\node\20.12.0\npm.CMD
Watchman: 2024041.112832.0 - C:\Users\Xantre\scoop\shims\watchman.EXE
IDEs:
Android Studio: AI-232.10227.8.2321.11479570
npmPackages:
react-native: * => 0.73.7
Expo Workflow: managed

Minimal reproducible example

https://github.com/XantreDev/no-hmr-env-repro

@XantreDev XantreDev added CLI Versioned Expo CLI -- `npx expo start` needs validation Issue needs to be validated labels May 15, 2024
@expo-bot expo-bot removed the needs validation Issue needs to be validated label May 15, 2024
@XantreDev
Copy link
Author

XantreDev commented May 15, 2024

Here is a patch that fixes an issue:

@expo__env@0.2.3.patch

diff --git a/build/env.js b/build/env.js
index f5e5e4efd8609d683d709219acf8b46554e62978..4c1bc0fc8baeae92fff946dcfd31cc60252f968f 100644
--- a/build/env.js
+++ b/build/env.js
@@ -138,24 +138,22 @@ function createControlledEnvironment() {
   function _expandEnv(parsedEnv) {
     const expandedEnv = {};
 
+    const processEnv = {...process.env}
+    for (const key in parsedEnv) {
+      delete processEnv[key]
+    }
     // Pass a clone of `process.env` to avoid mutating the original environment.
     // When the expansion is done, we only store the environment variables that were initially parsed from `parsedEnv`.
-    const allExpandedEnv = (0, _dotenvExpand().expand)({
+    (0, _dotenvExpand().expand)({
       parsed: parsedEnv,
-      processEnv: {
-        ...process.env
-      }
+      processEnv: processEnv
     });
-    if (allExpandedEnv.error) {
-      console.error(`Failed to expand environment variables, using non-expanded environment variables: ${allExpandedEnv.error}`);
-      return parsedEnv;
-    }
 
     // Only store the values that were initially parsed, from `parsedEnv`.
     for (const key of Object.keys(parsedEnv)) {
       var _allExpandedEnv$parse;
-      if ((_allExpandedEnv$parse = allExpandedEnv.parsed) !== null && _allExpandedEnv$parse !== void 0 && _allExpandedEnv$parse[key]) {
-        expandedEnv[key] = allExpandedEnv.parsed[key];
+      if ((_allExpandedEnv$parse = parsedEnv) !== null && _allExpandedEnv$parse !== void 0 && _allExpandedEnv$parse[key]) {
+        expandedEnv[key] = parsedEnv[key];
       }
     }
     return expandedEnv;

We need to allow keys to be overwritten, because existing keys is prefered https://github.com/motdotla/dotenv-expand/blob/ff7e2efd0d9925c7da82c167ae77d22a6c76e878/lib/main.js#L24C10-L31

@XantreDev
Copy link
Author

If you want I can provide a pr with this changes. But this do not fixes that change of env vars do not cause automatic server reload

@expo-bot
Copy link
Collaborator

Thank you for filing this issue!
This comment acknowledges we believe this may be a bug and there’s enough information to investigate it.
However, we can’t promise any sort of timeline for resolution. We prioritize issues based on severity, breadth of impact, and alignment with our roadmap. If you’d like to help move it more quickly, you can continue to investigate it more deeply and/or you can open a pull request that fixes the cause.

@byCedric
Copy link
Member

byCedric commented May 15, 2024

Hi @XantreDev! Thanks for the issue report.

If you want I can provide a pr with this changes.

Yeah, feel free to open a PR that applies the fix you mentioned. Let's consider the auto-reload, or HMR, issue separate from this one.

@XantreDev
Copy link
Author

Ok. @byCedric I will provide it today/tomorrow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Versioned Expo CLI -- `npx expo start` Issue accepted
Projects
None yet
Development

No branches or pull requests

3 participants