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

External packages not transforming process.env.NODE_ENV #9641

Closed
4 tasks done
mridgway opened this issue May 9, 2017 · 9 comments
Closed
4 tasks done

External packages not transforming process.env.NODE_ENV #9641

mridgway opened this issue May 9, 2017 · 9 comments

Comments

@mridgway
Copy link
Contributor

mridgway commented May 9, 2017

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

process.env.NODE_ENV checks are being shipped to the browser.

What is the expected behavior?

In React, there is a browserify transform that converts process.env.NODE_ENV to the actual environment variable.

In package.json of React:

  "browserify": {
    "transform": [
      "loose-envify"
    ]
  }

Packages

  • prop-types
  • create-react-class
  • react-addons-create-fragment
  • react-transition-group
@mridgway
Copy link
Contributor Author

mridgway commented May 9, 2017

@acdlite I can do a PR for create-react-class, but now I can't find the code for prop-types.

@gaearon
Copy link
Collaborator

gaearon commented May 9, 2017

It's in https://github.com/reactjs/prop-types.

@gaearon
Copy link
Collaborator

gaearon commented May 9, 2017

Does this mean we also need to do this for all addons? Or do they already have them?

@mridgway
Copy link
Contributor Author

mridgway commented May 9, 2017

I'll look through the addons to see. Only the addons that use process.env.NODE_ENV will need to have it and I think most are already proxying to require('react/foo').

@mridgway mridgway changed the title prop-types and create-react-class not transforming process.env External packages not transforming process.env.NODE_ENV May 9, 2017
@gaearon
Copy link
Collaborator

gaearon commented May 9, 2017

I think most are already proxying to require('react/foo').

They shouldn't—the point of 15.5 release was to decouple (almost all) addons so they can keep existing when React 16 removes /lib/.

@mridgway
Copy link
Contributor Author

mridgway commented May 9, 2017

I added a list to the description that shows which packages use process.env.NODE_ENV that don't include loose-envify. I can either add it to all of them or just the ones that use process.env.NODE_ENV. Up to you.

@gaearon
Copy link
Collaborator

gaearon commented May 9, 2017

Let's only add where necessary.

@gaearon
Copy link
Collaborator

gaearon commented May 10, 2017

create-react-class@15.5.3 and react-addons-create-fragment@15.5.4 should have the fixes.
Please let us know if this worked!

@gaearon gaearon closed this as completed May 10, 2017
@mridgway
Copy link
Contributor Author

Looks good. Thanks for the quick turnaround on these!

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

2 participants