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

Remove usage of `process.env.NODE_ENV` in ES module build #2907

Closed
keanulee opened this issue Mar 28, 2018 · 9 comments
Closed

Remove usage of `process.env.NODE_ENV` in ES module build #2907

keanulee opened this issue Mar 28, 2018 · 9 comments

Comments

@keanulee
Copy link

@keanulee keanulee commented Mar 28, 2018

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

Feature

What is the current behavior?

The ES module build of Redux references process.env.NODE_ENV which doesn't exist on browsers. With unpkg.com's ?module query param that converts package names to paths, this is the only issue blocking users from loading Redux as a module from the browser.

See references to process.env.NODE_ENV in:

What is the expected behavior?

No references to process.env.NODE_ENV in the ES module build.

Demo: http://jsbin.com/tazoliy/edit?html,output

@timdorr

This comment has been minimized.

Copy link
Member

@timdorr timdorr commented Mar 28, 2018

Note that React does this too.

This is important to prevent against the common mistake of including the development checks in a production build, as they significantly impact the speed of Redux. A workable solution would be to check for process's existence first. Can you make a PR for this?

@TimvdLippe

This comment has been minimized.

Copy link
Contributor

@TimvdLippe TimvdLippe commented May 5, 2018

I see that the corresponding PR has been closed for not being the appropriate solution, but this issue remains. Couls you reopen so we can discuss alternative solutions to resolve this?

@justinfagnani

This comment has been minimized.

Copy link

@justinfagnani justinfagnani commented May 5, 2018

Agreed @TimvdLippe. That Redux modules don't work in the browser without intervention is frustrating. This disrupts otherwise very nice flows like using unpkg.com to serve modules to jsbin and friends.

@markerikson

This comment has been minimized.

Copy link
Contributor

@markerikson markerikson commented May 5, 2018

Looking at #2910 , it seems that the proposed PR didn't correctly result in the blocks being removed from prod builds. I don't have time to pursue this myself, but if someone can contribute a working solution, we can consider getting it in.

@markerikson markerikson reopened this May 5, 2018
@markerikson

This comment has been minimized.

Copy link
Contributor

@markerikson markerikson commented May 5, 2018

(Note that the issue being "closed" certainly didn't stop people from being able to discuss this, but I'll re-open it as an indication that it's still a valid topic.)

@TimvdLippe

This comment has been minimized.

Copy link
Contributor

@TimvdLippe TimvdLippe commented Sep 24, 2018

EDIT: As the corresponding PR has been merged, please do not use the below method. Use the officially published Redux package instead

For anyone looking at a browser-compatible ES-module version, I have a PR open in #3143. However, until the Redux maintainers have decided whether or not to merge the PR, you can depend on https://github.com/TimvdLippe/redux/tree/es-browser-build to use the package. Add the following line to your package.json to get the correct version:

"dependencies": {
  "redux": "timvdlippe/redux.git#es-browser-build"
}
@rjsteinert

This comment has been minimized.

Copy link

@rjsteinert rjsteinert commented Oct 26, 2018

@TimvdLippe Do you have any examples you can point to where this is used? I'm still getting process is not defined using react 4.0.1 and also the package you recommended.

@TimvdLippe

This comment has been minimized.

Copy link
Contributor

@TimvdLippe TimvdLippe commented Oct 26, 2018

@rjsteinert You have to point to the es/redux.mjs specifically. You can not just do import "redux" and let that work. E.g. this should work: import { createStore } from "redux/es/redux.mjs";

@rjsteinert

This comment has been minimized.

Copy link

@rjsteinert rjsteinert commented Oct 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
You can’t perform that action at this time.