Define process.env as object #807
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
There was a problem hiding this comment.
This is not a correct fix.
The correct fix would be to use reduce to initialize the env object itself. So you'd use env[key] rather than env['process.env'][key] here, and you’d use NODE_ENV and PUBLIC_URL directly as keys below.
Then you would have the function return { 'process.env': env }. I think this would work.
| // This should only be used as an escape hatch. Normally you would put | ||
| // images into the `src` and `import` them in code to get their paths. | ||
| 'process.env.PUBLIC_URL': JSON.stringify(publicUrl) | ||
| 'process.env': { |
There was a problem hiding this comment.
You are overwriting this process.env key in the reduce callback.
|
Sorry @gaearon , please see my fix. Also, is that correct to update the NODE_EVN check in webpack.config.prod? https://github.com/facebookincubator/create-react-app/pull/807/files#diff-d2bddbd3bfb7051f9381474c844674faR52 |
|
Yes, this looks right! Thanks. |
…react-app # By Dan Abramov (5) and others # Via Dan Abramov * 'master' of https://github.com/facebookincubator/create-react-app: docs(readme): peer dependencies applied (facebook#818) Fix typos on ISSUE_TEMPLATE.md (facebook#817) Add explicit linebreaks (facebook#813) Fix typo (facebook#810) Fix some typos (facebook#809) Beaufity output of eject.js script (facebook#769) Define process.env as object (facebook#807) Typo fix in webpack.config.dev.js comments (facebook#777) Add Netlify to deploy instructions Fix usage example to match [email protected] API Relaxed eslint rule no-unused-expressions (facebook#724) Fix the doc Publish Add 0.6.1 changelog Moved Babel and ESLint config to package.json after ejecting (facebook#773) Conflicts: packages/react-scripts/package.json
|
Such approach can result in large development builds, if I think the better approach is to create definitions for both, so we get short aliases if possible and at the same time process.env is fully supported. |
|
Thanks for spotting. Can you raise an issue about this? |
* Define process.env as object * Fix define process.env * fix NODE_ENV check * Fix style nitpick
…react-app # By Dan Abramov (5) and others # Via Dan Abramov * 'master' of https://github.com/facebookincubator/create-react-app: docs(readme): peer dependencies applied (facebook#818) Fix typos on ISSUE_TEMPLATE.md (facebook#817) Add explicit linebreaks (facebook#813) Fix typo (facebook#810) Fix some typos (facebook#809) Beaufity output of eject.js script (facebook#769) Define process.env as object (facebook#807) Typo fix in webpack.config.dev.js comments (facebook#777) Add Netlify to deploy instructions Fix usage example to match [email protected] API Relaxed eslint rule no-unused-expressions (facebook#724) Fix the doc Publish Add 0.6.1 changelog Moved Babel and ESLint config to package.json after ejecting (facebook#773) Conflicts: packages/react-scripts/package.json
Issue: #795