search

angular-redux / ng-redux

Angular bindings for Redux
MIT License
1.16k stars 177 forks source link

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

Closed kaidjohnson closed 7 years ago

kaidjohnson commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@kaidjohnson

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

deini commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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 commented 6 years ago

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

deini commented 6 years ago

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