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

Fix HMR #865

Merged
merged 4 commits into from
Jun 8, 2017
Merged

Fix HMR #865

merged 4 commits into from
Jun 8, 2017

Conversation

Judahmeek
Copy link
Contributor

@Judahmeek Judahmeek commented Jun 7, 2017

So I've got hot-reloading working on spec/dummy, just not if both ReduxApp and ReduxSharedStoreApp are imported & registered (hot reloading works on both of them, just not both of them at once). I'm still working out why, but my assumption is that it has something to do with the dependency paths.

I also noticed that hot updates to ReduxSharedStoreApp would only apply to the second app on http://0.0.0.0:3000/client_side_hello_world_shared_store), I'm focusing on Egghead's lessoning editing form draft.


This change is Reviewable

@coveralls
Copy link

coveralls commented Jun 7, 2017

Coverage Status

Changes Unknown when pulling 75d401d on jmeek/fix-hmr into ** on master**.

@justin808
Copy link
Member

couple tiny thing, then pass CI and I will merge!


Reviewed 10 of 10 files at r1.
Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks broke.


spec/dummy/client/package.json, line 38 at r1 (raw file):

    "react-helmet": "^5.0.3",
    "react-hot-loader": "^3.0.0-beta.6",
    "react-on-rails": "file:../../../",

don't change that -- we use a relative path via yarn link


spec/dummy/client/server-rails-hot.js, line 16 at r1 (raw file):

// 3. Start up `foreman start -f Procfile.hot` to start both Rails and the hot reload server.

const webpack = require('webpack');

changed b/c babelrc must be modules false


spec/dummy/client/webpack.client.rails.hot.config.js, line 16 at r1 (raw file):

module.exports = merge.strategy(
  {
    entry: 'prepend'

worth commenting that this is b/c react-hot-loader needs to be first


spec/dummy/client/webpack.client.rails.hot.config.js, line 106 at r1 (raw file):

  plugins: [
    new webpack.HotModuleReplacementPlugin(),
    new webpack.NamedModulesPlugin(),

worth commenting on why if you want to leave in

good if it helps to troubleshoot!


spec/dummy/client/app/startup/ClientReduxApp.jsx, line 11 at r1 (raw file):

import { AppContainer } from "react-hot-loader";
import { render } from "react-dom";
import consoleErrorReporter from "lib/consoleErrorReporter";

might want to remove


spec/dummy/client/lib/renderApp.js, line 1 at r1 (raw file):

import React from 'react';

remove file if not used


Comments from Reviewable

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling f3fc297 on jmeek/fix-hmr into ** on master**.

1 similar comment
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling f3fc297 on jmeek/fix-hmr into ** on master**.

@justin808
Copy link
Member

:lgtm:

Looks ready to merge!


Reviewed 4 of 7 files at r2.
Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks broke.


Comments from Reviewable

@jeffling
Copy link

@justin808 Do you have any advice on setting up hot reload if we have around 20 entry points? (we use react only for dynamic parts of our page and templates for the rest.

@justin808
Copy link
Member

@Judahmeek can tell @jeffling.

@Judahmeek Judahmeek deleted the jmeek/fix-hmr branch September 14, 2017 08:24
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.

4 participants