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

fix(gatsby): allow environment variables to be replaced per environment #10565

Merged
merged 8 commits into from
Mar 14, 2019

Conversation

DSchau
Copy link
Contributor

@DSchau DSchau commented Dec 19, 2018

Description

This PR introduces a mechanism to source environment variables from an ACTIVE_ENV environment variable override. If the value is not set, the flow will be the same, e.g. .env.production will be used in gatsby build and gatsby serve and .env.development will be used in gatsby develop.

However, if an override variable ACTIVE_ENV is set, this will be used to source the .env file. For example, given ACTIVE_ENV=staging gatsby build, gatsby will attempt to source the .env.staging file rather than the the .env.production file.

Further info

I'm not really sold on ACTIVE_ENV so suggestions welcome. I don't really like the idea of exposing some other non-standard variable, but think this seems more maintainable and better than the alternatives (which I've outlined below in the related issue)

Related Issues

Fixes #10563

@DSchau DSchau requested a review from a team as a code owner December 19, 2018 21:18
@DSchau DSchau requested a review from a team December 19, 2018 21:18

describe(`environment variables`, () => {
afterEach(() => {
delete process.env.ACTIVE_ENV
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL that setting (and then unsetting via process.env.SOME_ENV = undefined) still makes that value available as a string, e.g.

process.env.ACTIVE_ENV = `whatever`

// later

process.env.ACTIVE_ENV = undefined

expect(process.env.ACTIVE_ENV).toBe(`undefined`) // true

@m-allanson
Copy link
Contributor

Maybe call this GATSBY_ACTIVE_ENV?

@m-allanson
Copy link
Contributor

m-allanson commented Dec 20, 2018

but think this seems more maintainable and better than the alternatives (which I've outlined below in the related issue)

I agree this seems like the best approach here.

(That e2e failure seems unrelated)

Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DSchau this looks ready to go. Please let me know if I'm wrong on this one!

@DSchau
Copy link
Contributor Author

DSchau commented Mar 14, 2019

@wardpeet let's do it :) Thanks!

@DSchau DSchau merged commit 828eaf8 into gatsbyjs:master Mar 14, 2019
@DSchau DSchau deleted the gatsby/env-override-fix branch March 14, 2019 15:13
@DSchau
Copy link
Contributor Author

DSchau commented Mar 14, 2019

Successfully published:

  • gatsby@2.1.33

@joefiorini
Copy link
Contributor

joefiorini commented Apr 11, 2019

Coming to this late, there are a couple issues open now because this PR causes GATSBY_ACTIVE_ENV to override NODE_ENV, which I can't imagine was intentional. I think it's a perfectly valid pattern to have two separate environment variables, one that names the environment in which the code lives and the other indicates the mode it should be built with. The latter is what we typically use NODE_ENV for, but the former doesn't really have a standard name yet. I worked on a project that heavily used Kubernetes and we called it ENVIRONMENT_NAME. ACTIVE_ENV works too. Either way, I think that the two should remain separate rather having one override the other. Anyone opposed to a followup PR changing this so that GATSBY_ACTIVE_ENV does not override NODE_ENV when it gets passed to DefinePlugin? (@wardpeet @DSchau)

@wardpeet
Copy link
Contributor

seems like a bug indeed! fixing

arcticicestudio added a commit to nordtheme/web that referenced this pull request Apr 25, 2019
This is the regular batch update for outdated production and development
dependencies.

The largest change is the migration to MDX 1.0.0 (1) using the official
migration guide for v0 to v1 (2).

React has been been updated to the latest patch version 16.8.6 (3) and
the `prop-types` package now comes with a handy new `elementType`
prop type (4) that can be used for React components.

`polished` has been updated to the large 3.0.0 version milestone (5)
that comes with many features in form of new modules, improvements like
a new error system as well as a a roadmap for v4.
The `readableColor` helper now offers the option to set the color(s) it
returns for light or dark colors instead of only returning `white` or
`black` based on the passed colors luminosity. `stripUnit` now offers
the option to return the value and unit as an array, replacing the
functionality of `getValueAndUnit` that'll is now deprecated and will be
removed in v4.
All color modules will now also safely handle the `transparent` keyword
instead of erroring out.
See the release notes for all details and changes.

React Waypoint has been updated to major version 9 (6) that comes with
improvements in library size and minifications in form of named exports
for the `Waypoint` module as well as for all defined constants.

Prettier 1.17.0 (7) now allows to use shared configurations making it
much easier to setup new projects and keep the config code base at one
single-point-of-truth. Also Markdown tables are now kept compact when
reformatting would exceed the print width (8) making largely improving
the readability.

Gatsby and all official plugins have been updated to the latest
versions. This comes with new features that allow environment variables
to be replaced per environment (9).

`gatsby-plugin-manifest` fixes the incorrect favicons size bug (10) that
often appeared as warning in the console.

`gatsby-plugin-sharp` now comes with a `defaultQuality` option (11) to
define the default quality for processed images instead of only allowing
to set the quality through the GraphQL query.

`gatsby-image` now comes with a `durationFadeIn` option (12) that
accepts a number instead of boolean to customize animation duration.

>>>>>> Production Dependencies

- @babel/polyfill `7.2.5` -> `7.4.3`
- @mdx-js/tag `0.18.0` -> `0.20.3`
- gatsby `2.0.75` -> `2.0.117`
- gatsby `2.1.4` -> `2.3.29`
- gatsby-image `2.0.30` -> `2.0.40`
- gatsby-mdx `0.4.0` -> `0.6.2`
- gatsby-plugin-canonical-urls `2.0.10` -> `2.0.12`
- gatsby-plugin-catch-links `2.0.10` -> `2.0.13`
- gatsby-plugin-google-gtag `1.0.13` -> `1.0.16`
- gatsby-plugin-lodash `3.0.4` -> `3.0.5`
- gatsby-plugin-manifest `2.0.17` -> `2.0.29`
- gatsby-plugin-netlify `2.0.9` -> `2.0.15`
- gatsby-plugin-offline `2.0.23` -> `2.0.25`
- gatsby-plugin-react-helmet `3.0.6` -> `3.0.12`
- gatsby-plugin-remove-trailing-slashes `2.0.7` -> `2.0.11`
- gatsby-plugin-sharp `2.0.23` -> `2.0.35`
- gatsby-plugin-sitemap `2.0.5` -> `2.0.12`
- gatsby-plugin-styled-components `3.0.5` -> `3.0.7`
- gatsby-plugin-svgr `2.0.1` -> `2.0.2`
- gatsby-source-filesystem `2.0.20` -> `2.0.32`
- gatsby-source-graphql `2.0.10` -> `2.0.18`
- gatsby-transformer-sharp `2.1.15` -> `2.1.18`
- gatsby-transformer-yaml `2.1.8` -> `2.1.12`
- inter-ui `3.3.2` -> `3.5.0`
- polished `2.3.3` -> `3.2.0`
- prop-types `15.6.2` -> `15.7.2`
- react `16.8.3` -> `16.8.6`
- react-dom `16.8.3` -> `16.8.6`
- react-pose `4.0.6` -> `4.0.8`
- react-spring `8.0.7` -> `8.0.19`
- react-waypoint `8.1.0` -> `9.0.2`
- semver `5.6.0` -> `6.0.0`
- styled-components `4.1.3` -> `4.2.0`
- typeface-rubik `0.0.54` -> `0.0.72`

>>>>>> Development Dependencies

- @babel/core `7.2.2` -> `7.4.3`
- @babel/plugin-proposal-class-properties `7.3.0` -> `7.4.0`
- @babel/plugin-proposal-nullish-coalescing-operator `7.2.0` -> `7.4.3`
- @mdx-js/mdx `0.20.1` -> `1.0.14`
- @mdx-js/tag `0.18.2` -> `0.20.3`
- @svgr/webpack `4.1.0` -> `4.2.0`
- babel-jest `24.1.0` -> `24.7.1`
- babel-plugin-react-remove-properties `0.2.5` -> `0.3.0`
- babel-preset-gatsby `0.1.7` -> `0.1.11`
- eslint `5.14.0` -> `5.16.0`
- eslint-plugin-import `2.16.0` -> `2.17.2`
- eslint-plugin-react-hooks `1.0.2` -> `1.6.0`
- husky `1.3.1` -> `2.1.0`
- jest `24.1.0` -> `24.7.1`
- jest-dom `3.0.2` -> `3.1.3`
- jest-junit `6.2.1` -> `6.3.0`
- lint-staged `8.1.3` -> `8.1.5`
- prettier `1.16.4` -> `1.17.0`
- react-testing-library `5.5.3` -> `6.1.2`
- webpack-bundle-analyzer `3.0.3` -> `3.3.2`

References:
  (1) https://mdxjs.com/blog/v1
  (2) https://mdxjs.com/migrating/v1
  (3) https://github.com/facebook/react/releases/tag/v16.8.6
  (4) facebook/prop-types#211
  (5) https://github.com/styled-components/polished/releases/tag/v3.0.0
  (6) https://github.com/brigade/react-waypoint/releases/tag/v9.0.0
  (7) https://prettier.io/blog/2019/04/12/1.17.0.html
  (8) https://prettier.io/blog/2019/04/12/1.17.0.html#do-not-align-table-contents-if-it-exceeds-the-print-width-and-prose-wrap-never-is-set-5701-by-chenshuai2144
  (9) gatsbyjs/gatsby#10565
  (10) gatsbyjs/gatsby#12081
  (11) gatsbyjs/gatsby@8af9826
  (12) gatsbyjs/gatsby#13566

GH-137
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

Successfully merging this pull request may close these issues.

Environment variables are not captured utilizing NODE_ENV in Webpack
5 participants