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 HMRE and allow vanilla HMR #192

Closed
wants to merge 3 commits into from

Conversation

imranismail
Copy link
Contributor

This should close #188

@lukekarrys
Copy link
Contributor

I don't think we should remove the ability to use babel-preset-react-hmre or babel-plugin-react-transform (+ the other connected modules) out of the box. They can still useful in some cases I think.

Could this be updated to keep those, but still allow vanilla HMR? The way I envisioned it, if the proper loaders were not installed the function would return early but not reset config.devServer.hot = false in case the user still wanted to use HMR in some other way.

@imranismail
Copy link
Contributor Author

Sure! I'll try to come up with something tonight

@imranismail
Copy link
Contributor Author

If we don't reset the config.devServer.hot = false we still need to check for redbox-react and babel-preset-react. Plus plugins has to include new webpack.HotModuleReplacementPlugin()

@HenrikJoreteg
Copy link
Owner

Perhaps we should just leave HMR on all the time as part of the base, people could explicitly turn it off if they want, but I feel like that would be the exception, not the rule.

@HenrikJoreteg
Copy link
Owner

I mean, a big part of the reason people use this... at least I think, is to get free style hotloading just working out of the box without the need for running browser plugins. Seems like a logical default to leave on and then if people really wanted to turn it off they could configure it directly by passing a devServer option.

@lukekarrys
Copy link
Contributor

Perhaps we should just leave HMR on all the time

Yeah, I agree. With the number of different ways and reasons people would use HMR I think we should remove the checking of which loaders/plugins are installed, and instead just add the entry and HotModuleReplacementPlugin if config.hot is set to true (which it is by default).

@imranismail
Copy link
Contributor Author

@lukearrys something like this?

var webpack = require('webpack')
var path = require('path')

function load (config) {
  // add hot loading clientside code
  config.entry.unshift(
    // Full path to webpack-hot-middleware so it works in npm2 and npm3
    path.join(path.dirname(require.resolve('webpack-hot-middleware')), 'client')
  )

  // add dev plugins
  config.plugins = config.plugins.concat([
    new webpack.HotModuleReplacementPlugin()
  ])
}

module.exports = {
  load: load
}

@lukekarrys
Copy link
Contributor

@imranismail Looks good to me! 👍

@lukekarrys
Copy link
Contributor

@imranismail Also after reading this comment and the short term fixes here I think it'd be best to leave in the NoErrorsPlugin by default.

@imranismail
Copy link
Contributor Author

I've submitted the changes in another PR, gonna leave this fork untouched for now.

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.

Allow vanilla hmr
3 participants