Skip to content

Merge dev and prod to index in store #182

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

Merged
merged 1 commit into from
Jul 8, 2016
Merged

Merge dev and prod to index in store #182

merged 1 commit into from
Jul 8, 2016

Conversation

ngtan
Copy link
Contributor

@ngtan ngtan commented Jul 8, 2016

Something I see an error "No default export found in module import/default" when I import configureStore into app/index.js
but the application is still running. So, I think we should merge config dev and config store code into index file.
And another problem, if have dev and prod file, we must code in 2 file. I think I spent a lot of time for the problem.

@coveralls
Copy link

coveralls commented Jul 8, 2016

Coverage Status

Coverage remained the same at 98.906% when pulling 68d54f3 on ngtan:master into 2e45af5 on coryhouse:master.

@coryhouse coryhouse merged commit feb88b5 into coryhouse:master Jul 8, 2016
@coryhouse
Copy link
Owner

@ngtan Good points. Thanks for the pull request!

@coryhouse
Copy link
Owner

Whoops, correction. I just rolled this back. The reason these are separate files is to assure that no dev specific code is included in the production bundle. With this pull request, development specific code like thunk/saga/reduxImmutableStateInvariant, etc would be included in the production bundle. Why? Because ES6 imports are statically analyzable. So the import at the top of a single configureStore.js file would have to include development-specific resources that we don't want in production.

The real solution to your problem: Make sure index.js has this on line 1 (as reflected in the repo):
/* eslint-disable import/default */

@ngtan
Copy link
Contributor Author

ngtan commented Jul 8, 2016

I got it. Thanks Cory.

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.

3 participants