Menardi commented 3 years ago

Describe the bug

Native apps always use the variables from index.web.js, rather than the native variables, even if js_override is not set.

Steps to Reproduce

Create two environments, .env.development with APP_NAME=Development and .env.production with APP_NAME=Production
Create a simple app which shows the app's name in a <Text> component
Build and run the development app: yarn rnuc .env.development && yarn android
Note the text component shows Development
Run yarn rnuc .env.production
Note the text hot reloads and now shows Production

Editing the config in node_modules/react-native-ultimate-config/index.web.js will also cause the text to change, showing that the native app is taking the value from here, instead of from the native configuration.

Expected behavior

The environment variables should only change when a new native build is done.

Environment

Integration option

[x] I am using rnuc CLI to inject environment (default integration option from quickstart guide)
[ ] I am using scheme/flavor based integration (experimental integration option from cookbook: scheme, flavors)
[ ] I am using react-native-ultimate-config within monorepo managed with Lerna or Yarn Workspaces Monorepo support

Env file

[x] I am using dotenv file as a source for variables
[ ] I am using YAML file as a source for variables

Example of env file that is causing problems

APP_NAME=Development

OS

[x] ios
[x] android

Packages

version of react-native: ❗ 0.65.1
version of react-native-ultimate-config: ❗3.4.1

Additional context

Metro, the React Native bundler, prefers browser to main entries in package.json, meaning that a native app will always take react-native-ultimate-config's index.web.js file. However, it supports a react-native key, which takes priority over both of these (see https://github.com/facebook/metro/blob/7e99d9eb673469f88d0c03544f4533b4f56f75f9/packages/metro/src/ModuleGraph/node-haste/Package.js#L108-L116 for the implementation of that).

I think the fix should be quite simple, just adding one line to the package.json:

"react-native": "index.js",

I will create a PR for this, but I have never used react-native-web before, so I don't know if its bundler will prefer a browser or react-native entry in the package.json. I presume it would choose browser, but I don't know for sure.

maxkomarychev commented 3 years ago

hey, thanks for the report and investigation

the fix makes sense to me, looking forward to see the PR!

thanks!