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

Refactor, css-modules, spring, hot module replacement and fixes #219

Closed
wants to merge 0 commits into from
Closed

Refactor, css-modules, spring, hot module replacement and fixes #219

wants to merge 0 commits into from

Conversation

gauravtiwari
Copy link
Member

@gauravtiwari gauravtiwari commented Apr 1, 2017

This one is not merged

@aganov
Copy link
Contributor

aganov commented Apr 1, 2017

@gauravtiwari I'm just wondering should we have to force usage of css-modules. Currently, due to modified asset_pack_path helper there is no option to set modules: false and just use ExtractTextPlugin in dev mode. Maybe making it optional will be a good idea?

@mfazekas
Copy link

mfazekas commented Apr 2, 2017

It seems that this PR enables HMR by default, this can be confusing for example if you have js console open in chrome as it will break in exception

image

accepting updates to hmr is non trivial thing. You need to deregister old version of your event listeners, and reregister with new version, etc.

I'd liked the old behaviour better where hmr was disabled by default, and whoever needed it could start with ./bin/webpack-dev-server --hot

@matthewd
Copy link
Member

matthewd commented Apr 2, 2017

This PR covers multiple issues that has been opened lately

Can we please not do that again? #153 effectively became the project's master branch for several weeks, vetoing all other contributions.

@ytbryan
Copy link
Contributor

ytbryan commented Apr 2, 2017

Once again, thank you for the contribution @gauravtiwari

Perhaps you can split it up into multiple PRs. For example, the one that check yarn and node version could be separate out. :)

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