Skip to content

Prevent error message about NODE_ENV not being 'production' when using ng-redux in the browser #126

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

Closed
wants to merge 1 commit into from

Conversation

kaidjohnson
Copy link

When including ng-redux.min.js in a script tag on a site, the included package throws a message (from redux src/index.js):

    20     'You are currently using minified code outside of NODE_ENV === \'production\'. ' +                                                                                                
    21     'This means that you are running a slower development build of Redux. ' +                                                                                                         
    22     'You can use loose-envify (https://github.com/zertosh/loose-envify) for browserify ' +                                                                                            
    23     'or DefinePlugin for webpack (http://stackoverflow.com/questions/30030031) ' +                                                                                                    
    24     'to ensure you have the correct code for your production build.'            

To avoid this error, we need to webpack ng-redux with the proper 'production' environment to ensure the minified code can be used in a standalone <script> tag. I have modified the npm build:min task to do so, which appears to work. At the moment, we get a package that is 3k smaller, but webpack also complains a bit about unreachable code. It doesn't appear to be an issue, and ng-redux continues to work as expected, now without the above warning thrown in the console.

There may be a more elegant way of doing this with a webpack config file to avoid the spitting of warnings during webpacking, but for now, this solution does the job for us.

@kaidjohnson
Copy link
Author

I went down the path of #147 at first, too, however I realize that only masked the issue. The real problem is the we're bundling Redux inside of ng-redux, instead of letting ng-redux work with the Redux library provided on the page. This internal bundling is a bad practice in my opinion, as Redux may be used outside of ng-redux and would have to be re-included separately to do so, since it's not made available from the ng-redux library, causing duplication of a framework dependency.

@AntJanus
Copy link
Collaborator

@kaidjohnson

@deini and I were just discussing that. There is a consideration to include redux as a peer dependency and skip bundling it.

@deini
Copy link
Collaborator

deini commented Aug 16, 2017

@kaidjohnson I have a POC but will be addressed as a breaking change so probably until 4.x.x but you are completely right as in bundling redux as part of the lib is a bad practice.

@kaidjohnson
Copy link
Author

@deini Sounds good, thanks! Just as a reference, this ticket includes a working POC as well; we've actually been using my forked version of this lib in production for this very reason.

Keep us posted, we'd love to get back onto the official package as soon as this feature is available!

@deini
Copy link
Collaborator

deini commented Sep 6, 2017

@kaidjohnson There are a couple of builds that we expose now, we have an es build, a commonjs and a umd.

The error should be fixed in all of them, however umd still bundles Redux.

@MarSoft
Copy link

MarSoft commented Jan 4, 2018

Looks like @paulcookie/ng-redux build does not have this problem.

@deini
Copy link
Collaborator

deini commented Jan 4, 2018

@MarSoft This has been resolved for a while, are you on the latest version?

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.

5 participants