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

Migrate react preset and create-project to properly use RHL v4 #902

Merged
merged 4 commits into from Jun 1, 2018
Merged

Migrate react preset and create-project to properly use RHL v4 #902

merged 4 commits into from Jun 1, 2018

Conversation

eliperelman
Copy link
Member

@eliperelman eliperelman commented May 23, 2018

No description provided.

@eliperelman eliperelman requested a review from May 23, 2018
@edmorley
Copy link
Member

@edmorley edmorley commented May 25, 2018

(For reviewing reference) Migration guide:
https://github.com/gaearon/react-hot-loader#migrating-from-v3

Copy link
Member

@edmorley edmorley left a comment

Yey for more things updated! 😃

@@ -10,16 +10,21 @@ module.exports = (neutrino, opts = {}) => {
const mode = neutrino.config.get('mode');
const options = merge({
hot: true,
hotEntries: [require.resolve('react-hot-loader/patch')],
Copy link
Member

@edmorley edmorley May 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are now no default hotEntries in any of the presets that inherit web (or in web itself). Should we now remove hotEntries from the web preset entirely?

Copy link
Member Author

@eliperelman eliperelman May 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking so.


For example:
For example, if your `src/index` main entry renders a root component from
`App.jsx`, then the exported component needs to have a hot export:
Copy link
Member

@edmorley edmorley May 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a link to the RHL docs?

Copy link
Member Author

@eliperelman eliperelman May 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that would be good.

@edmorley edmorley added this to the v9 milestone May 25, 2018
@eliperelman
Copy link
Member Author

@eliperelman eliperelman commented May 30, 2018

I don't want to land this until #917 lands and I can bump the RHL major version installed in create-project.

@@ -219,17 +218,6 @@ module.exports = (neutrino, opts = {}) => {
config.devtool('cheap-module-eval-source-map');
config.when(options.hot, () => {
neutrino.use(hot);
config.when(options.hotEntries, (config) => {
Copy link
Member

@edmorley edmorley May 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add a check for hotEntries so the user knows to clean up their config (and also why things aren't working if they rely on it) - like we do for the deprecated options.minify options above.

Copy link
Member Author

@eliperelman eliperelman May 30, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call.

@eliperelman
Copy link
Member Author

@eliperelman eliperelman commented May 31, 2018

Updated PR to upgrade create-project's install of RHL to v4.

@edmorley edmorley merged commit e780e25 into neutrinojs:master Jun 1, 2018
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants