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

7.4.0 is breaking create-react-app projects that worked in prior versions #168

Closed
mdesousa opened this issue Aug 28, 2023 · 11 comments
Closed

Comments

@mdesousa
Copy link

mdesousa commented Aug 28, 2023

Hi, thanks for this great project! We have found it very useful over the years and use it everywhere that needs to deal with environment variables. It appears that 7.4.0 breaks projects that use create-react-app... seems related to #115 (Edit by Evan: I think this was meant to be #155).

I understand the motivation of making the library usable for vite (we use it for new projects, and it's great that it's supported). But this shouldn't be at the expense of breaking create-react-app... is there a way to default to the behavior or reading from process.env and allow vite projects to override?

The documented workaround (see below) isn't great... it means that we have to explicitly add our environment variables everywhere that they are imported. This is a lot of maintenance every time that we add or remove variables. Thanks.

const env = from({
  SOME_VARIABLE: process.env.REACT_APP_SOME_VARIABLE
});
@mdesousa
Copy link
Author

As a side note... it's also challenging for libraries that might be shared in the frontend and the backend. Before they were able to rely on a common abstraction process.env that was available in both node and create-react-app.
With this change that's not possible anymore and you would need different code to read environment variables for the frontend and backend.

@evanshortiss
Copy link
Owner

evanshortiss commented Aug 28, 2023

@mdesousa can you provide the syntax you were using before, that is now broken? My understanding is that this library didn't "just work" with create-react-app, and the example code you shared was always necessary (because that's how create-react-app works with process.env, and not at all specific to this library)

@evanshortiss
Copy link
Owner

I just tested with the latest version of create-react-app (13.4.19) by adding:

import { get } from 'env-var'

to the layout.tsx and then adding:

<title>{get('REACT_APP_TITLE').default('FOO').asString()}</title>

I then started the application using npm run dev and the page title was FOO. Starting with REACT_APP_TITLE=BAR npm run dev changed the page title to BAR. So things are working as expected, no? In fact, this seems to work better than before since the code you shared used to be necessary or the variables would not be injected correctly.

Can you make sure to provide what version of create-react-app you're using and the specific syntax?

@izahid19
Copy link

switch this to vite + react

@mdesousa
Copy link
Author

hi @evanshortiss , i just tested and was able to reproduce the issue. please take a look at this repo. it has two commits:

  • the first commit is [email protected]. if you switch to that commit and run npm start you'll see the value from the variable is read as expected.
  • the second commit is [email protected]. if you switch to that commit, run npm install, and npm start you'll see that the variable is not read and an "ERROR" message is shown in the sample app.

@evanshortiss
Copy link
Owner

evanshortiss commented Aug 28, 2023

Thanks so much for the repro repo. I see what you mean. It's related to the way the older create-react-app's (CRA's) polyfill is implemented.

Can you test a fix for me? Try npm i evanshortiss/env-var#cra-fix -S, then rm -rf node_modules/.cache (I had to do this to force CRA to load the new version of env-var) and run npm start and see if that works. It's working for me here.

EDIT: Ugh, I just realised this will break Vite, and others but it's a start on a hacky fix I guess.

Details

This is how the bundled env-var.js file looks with [email protected] when processed by CRA:

module.exports = from(({"NODE_ENV":"development","PUBLIC_URL":"","WDS_SOCKET_HOST":undefined,"WDS_SOCKET_PATH":undefined,"WDS_SOCKET_PORT":undefined,"FAST_REFRESH":true,"REACT_APP_TEXT":"Learn Env-Var"}));

Here's how it looks with [email protected]:

module.exports = from(typeof process === 'undefined' ? {} : ({"NODE_ENV":"development","PUBLIC_URL":"","WDS_SOCKET_HOST":undefined,"WDS_SOCKET_PATH":undefined,"WDS_SOCKET_PORT":undefined,"FAST_REFRESH":true,"REACT_APP_TEXT":"Learn Env-Var"}));

The polyfill seems to replace process.env with that Object instead of injecting a process object in the given scope that looks like process = { env: ENV_CONTENTS }.

I assume reason it works with the newer version of create-react-app is because they' better engineered the polyfill.

@mdesousa
Copy link
Author

That works for me @evanshortiss! Thanks for taking a look. By the way, i'm not sure what you mean by "newer version of create-react-app"? The repo that I shared used the latest... 5.0.1. Same version referenced in https://create-react-app.dev/. You mentioned version 13.4.19... where did you get that?

@evanshortiss
Copy link
Owner

evanshortiss commented Aug 28, 2023

@mdesousa interesting...my create-react-app example that I created via npx create-next-app@latest was using next in the scripts instead of react-scripts. I wonder if it's related to options passed to it?

EDIT: I see my error...I used my command history. Ignore my silliness 😫

┬─[eshortis@mbp:/tmp]─[14:59:16]
╰─>$ npx create-next-app@latest
✔ What is your project named? … cra-test
✔ Would you like to use TypeScript? … No / Yes
✔ Would you like to use ESLint? … No / Yes
✔ Would you like to use Tailwind CSS? … No / Yes
✔ Would you like to use `src/` directory? … No / Yes
✔ Would you like to use App Router? (recommended) … No / Yes
✔ Would you like to customize the default import alias? … No / Yes
Creating a new Next.js app in /private/tmp/cra-test.

Version comes from:

npx create-next-app@latest --version
13.4.19

See my comment about the fix. I need to think about how to get that fix in, but not break Vite...

@evanshortiss
Copy link
Owner

@mdesousa @izahid19 I published 7.4.1. I believe it retains the Vite compat added in 7.4.0, but fixes issues with the create-react-app polyfill for process.env. LMK if you run into any other issues.

@mdesousa
Copy link
Author

oh @evanshortiss i see... create-next-app is very different from create-react-app. That clarifies that...
we tested 7.4.1 and that solves the issue, thanks again for your help 👍

@evanshortiss
Copy link
Owner

Yeaaaah! I had used create-next-app more recently and used command completion without paying attention 🤦 Thanks so much for confirming 7.4.1. is working for you 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants