facebook / create-react-app

Set up a modern web app by running one command.
https://create-react-app.dev
MIT License
102.52k stars 26.77k forks source link

replace dotenv with dotenv-webpack? #6642

Open Nickman87 opened 5 years ago

Nickman87 commented 5 years ago

Is this a bug report?

No

Description

We are using a .env file to make configuration changes easy between our different environments and it works great. However, the way that dotenv works it not particularly interesting as it:

  1. bloats the code (even minified)
  2. exposes all environment variables right next to each other (the values will become public anyway, but you don't need them to be right next to each other)

In our codebase, we have a couple of variables and this block is inserted six times into our code: Image of Yaktocat

In a project without create-react-app I've been using dotenv-webpack with great success. It effectively replaces your call to an environment variable with the value itself, minimizing code bloat.

I believe this would be a small change in the code, but I was wondering if there are reasons not to do this? If there is no reason not to switch it, I would like to work on a PR.

heyimalex commented 5 years ago

Just chiming in: it looks like you're using process.env dynamically, which is hard to resolve at compile time and is probably why it's including the whole thing? I don't have anything like this in my bundle. Maybe you have a function like this.

function featureIsEnabled(feature: string): boolean {
  return !!feature && process.env["REACT_APP_FEATURE_"+feature.toUpperCase()] === "true";
}
Nickman87 commented 5 years ago

@heyimalex It took a while for me to get back to you but I finally found the time to do so.

You were right, I did have some dynamic reads like that in the code. I've now removed them and replaced them with static calls, but I still get loads of full listings in my final build.

ex.

var version = "".concat(Object({
    "NODE_ENV": "XXXX",
    "PUBLIC_URL": "XXXX",
    "REACT_APP_AWS_REGION": "XXXX",
    "REACT_APP_AWS_USERPOOL": "XXXX",
    "REACT_APP_AWS_CLIENTAPP": "XXXX",
    "REACT_APP_ZENDESK_KEY": "XXXX",
    "REACT_APP_ZENDESK_KEY_API": "XXXX",
    "REACT_APP_ZENDESK_URI": "XXXX",
    "REACT_APP_RECAPTCHA_SITE_KEY": "XXXX",
    "REACT_APP_COOKIE_DOMAIN": "XXXX",
    "REACT_APP_REFERRED_KEY": "XXXX",
    "REACT_APP_REFERRED_VALUE": "XXXX",
    "REACT_APP_COGNITO_BASE_URL": "XXXX",
    "REACT_APP_API_BASE_URL": "XXXX",
    "REACT_APP_SUPPORT_API_BASE_URL": "XXXX",
    "REACT_APP_TICKETING_API_BASE_URL": "XXXX",
    "REACT_APP_EVENT_API_BASE_URL": "XXXX",
    "REACT_APP_QUEUE_BASE_URL": "XXXX",
    "REACT_APP_ENV": "XXXX",
    "REACT_APP_FEATURE_TICKETING": "XXXX",
    "REACT_APP_FEATURE_TICKETING_UNAVAILABLE": "XXXX",
    "REACT_APP_FEATURE_BRACELETS": "XXXX",
    "REACT_APP_FEATURE_DEV": "XXXX",
    "REACT_APP_BEASTMODE": "XXXX",
    "REACT_APP_ATTRIBUTES_API_BASE_URL": "XXXX",
    "REACT_APP_GOOGLE_ANALYTICS": "XXXX",
    "REACT_APP_FACEBOOK_PIXEL": "XXXX",
    "REACT_APP_ALLOW_INSECURE_COOKIES": "XXXX",
    "REACT_APP_CONFIG_URL": "XXXX",
    "REACT_APP_INTELLITIX_W1_BASE_URL": "XXXX",
    "REACT_APP_INTELLITIX_W2_BASE_URL": "XXXX",
    "REACT_APP_FALLBACK_URL": "XXXX",
    "REACT_APP_REDIRECT_WHITELIST": "XXXX",
}).REACT_APP_VERSION || '0.0.0', "-").concat(REACT_ENV);

--> original

const version = `${process.env.REACT_APP_VERSION || '0.0.0'}-${REACT_ENV}`

Another:

pc(e, Object({
  NODE_ENV: "XXXX",
  PUBLIC_URL: "XXXX",
  REACT_APP_VERSION: "XXXX",
  REACT_APP_FEATURE_TICKETING: "XXXX",
  REACT_APP_FACEBOOK_PIXEL: "XXXX",
  REACT_APP_SUPPORT_API_BASE_URL: "XXXX",
  REACT_APP_RECAPTCHA_SITE_KEY: "XXXX",
  REACT_APP_FALLBACK_URL: "XXXX",
  REACT_APP_ENV: "XXXX",
  REACT_APP_REFERRED_VALUE: "XXXX",
  REACT_APP_AWS_REGION: "XXXX",
  REACT_APP_AWS_CLIENTAPP: "XXXX",
  REACT_APP_EVENT_API_BASE_URL: "XXXX",
  REACT_APP_INTELLITIX_W1_BASE_URL: "XXXX",
  REACT_APP_GOOGLE_ANALYTICS: "XXXX",
  REACT_APP_API_BASE_URL: "XXXX",
  REACT_APP_FEATURE_BRACELETS: "XXXX",
  REACT_APP_INTELLITIX_W2_BASE_URL: "XXXX",
  REACT_APP_BEASTMODE: "XXXX",
  REACT_APP_COGNITO_BASE_URL: "XXXX",
  REACT_APP_CONFIG_URL: "XXXX",
  REACT_APP_ALLOW_INSECURE_COOKIES: "XXXX",
  REACT_APP_ZENDESK_KEY: "XXXX",
  REACT_APP_QUEUE_BASE_URL: "XXXX",
  REACT_APP_COOKIE_DOMAIN: "XXXX",
  REACT_APP_AWS_USERPOOL: "XXXX",
  REACT_APP_REFERRED_KEY: "XXXX",
  REACT_APP_ZENDESK_URI: "XXXX",
  REACT_APP_FEATURE_TICKETING_UNAVAILABLE: "XXXX",
  REACT_APP_TICKETING_API_BASE_URL: "XXXX",
  REACT_APP_ATTRIBUTES_API_BASE_URL: "XXXX",
  REACT_APP_REDIRECT_WHITELIST: "XXXX",
  REACT_APP_FEATURE_DEV: "XXXX",
  REACT_APP_ZENDESK_KEY_API: "XXXX",
}).REACT_APP_ALLOWED_CALLBACK_DOMAINS)

--> original

isCallbackUrlAllowed(
        callbackUrl,
        process.env.REACT_APP_ALLOWED_CALLBACK_DOMAINS
)

These don't seem like very special cases to me? Any ideas of what is causing this?

Nickman87 commented 5 years ago

After looking into this further, I have noticed that only the places where an environment variable is used that has not been defined in the .env file get a full object listing like this. So it does not seem like a dotenv issue by iself.

I do wonder why this is the "minified" version of the code, the compiler should be able to know that this value is undefined? Any ideas on how or where to report this to get it fixed?

heyimalex commented 5 years ago

I agree that that's a bug. I'll try and repro sometime soon, but if you want to dig in it looks like we use webpack's DefinePlugin.

AbraaoAlves commented 4 years ago

@Nickman87 I did a PR to try to fix this behavior but without feedback ... Can you test my branch to see it's still happening?