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

How to detect/enable HMR for objects passed into functions/instances and extended without doing some really hacky stuff? #1910

Closed
ghost opened this issue Jan 21, 2016 · 15 comments
Labels

Comments

@ghost
Copy link

ghost commented Jan 21, 2016

For example, suppose you have ./templates/a.js:

export default {
  a: 'apple'
};

And then suppose some other module passes that to the arguments of some constructor in ./main.js:

import { render } from 'react-dom';
import App from './components/App';
import a from './templates/a';

render(<App template={a} />, document.getElementById('root'));

And ./components/App.js:

import React, { Component } from 'react';

export default class App extends Component {
  constructor(props) {
    super(props);

    this.stuff = {
      ...props.template,
      b: 'banana'
    };
  }

  render() {
    return (
      <div>
        <a>{this.stuff.a}</a>
        <b>{this.stuff.b}</b>
      </div>
    );
  }
}

And so with that example (admittedly kind of a ridiculous one, but bear with me) I want to be able to edit ./templates/a.js and have those changes be reflected within the App instance. Is there a clean way to do that?

@sokra
Copy link
Member

sokra commented Jan 23, 2016

The module which imports the changed module need to accept it.

Example:

(Note: babel rewrites import so you need to use require in webpack 1)

import { render } from 'react-dom';
import App from './components/App';
var a = require('./templates/a').default;

function renderRoot() {
  render(<App template={a} />, document.getElementById('root'));
}
renderRoot();

if(module.hot) {
  module.hot.accept("./template/a", function() {
    a = require("./template/a").default;
    renderRoot();
  });
}

webpack 2 Example:

import { render } from 'react-dom';
import App from './components/App';
import a from './templates/a';

function renderRoot() {
  render(<App template={a} />, document.getElementById('root'));
}
renderRoot();

if(module.hot) {
  module.hot.accept("./template/a", renderRoot);
}

@ghost
Copy link
Author

ghost commented Jan 23, 2016

Ah that must be why it didn't work for me with webpack 1. I was using import instead of require. I'll try webpack 2 then, but I have another quick/related question.

What if we have an index containing all of the templates as an object, ./templates/index.js:

import a from './a';
import b from './b';
import c from './c';

export default {
  a,
  b,
  c
};

And then we have ./defaultProps.js:

import templates from './templates/index';

export default {
  templates
};

And then in ./main.js:

import { render } from 'react-dom';
import App from './components/App';
import defaultProps from './defaultProps';

export function renderRoot(props = defaultProps) {
  return render(<App { ...props } />, document.getElementById('root'));
}
renderRoot();

if (module.hot) {
  module.hot.accept("./defaultProps", renderRoot); // ???
}

To make all that hot reloadable, would all the modules and their dependencies within ./defaultProps.js need module.hot.accept()? Or would I need to do something like:

if (module.hot) {
  const hotModules = ['./defaultProps', './templates/index'].concat(
    Object.keys(defaultProps.templates).map(key => (`./templates/${key}`))
  );

  module.hot.accept(hotModules, renderRoot);
}

@sokra
Copy link
Member

sokra commented Jan 23, 2016

Only the direct required module can be accepted. The updates bubble up to parents when the module is not accepted. So you only need to accept ./defaultProps can this handles updates to all nested modules.

@ghost
Copy link
Author

ghost commented Jan 25, 2016

I can't seem to get this to work.

import React from 'react';
import { render } from 'react-dom';
import App from './components/App';
const defaultProps = require('./defaultProps').default;

const getRootElement = () => document.getElementById('root');
const providedState = window.clientState;
let appInstance = null;

export default function renderApp(props, element = getRootElement()) {
  appInstance = render(<App { ...props } />, element);
  return appInstance;
}

renderApp({ ...defaultProps, providedState });

if (process.env.NODE_ENV !== 'production') {
  if (module.hot) {
    let currentProviders = defaultProps.providers;

    module.hot.accept('./defaultProps', () => {
      console.log('--> this never gets logged when something is changed in defaultProps.js <--');
      if (currentProviders !== defaultProps.providers) {
        currentProviders = defaultProps.providers;
        renderApp({ ...appInstance.props, providers: currentProviders });
      }
    });
  }
}

@ghost
Copy link
Author

ghost commented Jan 26, 2016

Also, I'm using webpack-hot-middleware and as far as I can tell, I'm doing everything you suggested for webpack 1. I've tried a few different things and it has never called the callback passed to module.hot.accept. No idea what I'm doing wrong. Should I dig into webpack's code and debug?

@ghost
Copy link
Author

ghost commented Jan 26, 2016

@glenjamin Do you know if the types of examples in this issue work for sure with webpack-hot-middleware?

@glenjamin
Copy link

There is nothing different about hot middleware in this regard. Re-rendering the root component is a common pattern.

I've not tested with webpack v2, so I have no idea if the APIs are the same.

@ghost
Copy link
Author

ghost commented Jan 26, 2016

I've spent the past few hours trying to narrow down the issue but I can't seem to find it. Perhaps I'm completely overlooking something.

The dev server is created using the following middleware:

var compiler = webpack(config);
var devMiddleware = webpackDevMiddleware(compiler, {
  noInfo: true,
  publicPath: config.output.publicPath
})
var hotMiddleware = webpackHotMiddleware(compiler);

The webpack dev config looks about like this:

module.exports = {
  devtool: 'cheap-module-eval-source-map',
  entry: {
    App: [
      'webpack-hot-middleware/client',
      './src/renderApp.js'
    ],
    DarkTheme: './src/themes/dark/index.js',
    LightTheme: './src/themes/light/index.js'
  },
  resolve: {
    extensions: ['', '.js']
  },
  output: {
    path: path.join(__dirname, 'dist'),
    filename: '[name].js',
    chunkFilename: '[id].js',
    publicPath: '/dist/',
    library: '[name]',
    libraryTarget: 'umd'
  },
  plugins: [
    new webpack.optimize.OccurrenceOrderPlugin(),
    new webpack.HotModuleReplacementPlugin(),
    new webpack.NoErrorsPlugin(),
    new webpack.DefinePlugin({
      'process.env.NODE_ENV': JSON.stringify('development')
    }),
    new ExtractTextPlugin('[name].css')
  ],
  module: {
    loaders: [
      // babel, etc.
    ]
  }
};

The client is essentially served:

<script src="/dist/App.js"></script>

I've reduced renderApp.js to this:

var defaultProps = require('./defaultProps');

console.log('this gets logged on the client');
console.log(defaultProps);

if (process.env.NODE_ENV !== 'production') {
  if (module.hot) {
    console.log('and this gets logged on the client');
    console.log(defaultProps);

    module.hot.accept('./defaultProps', function() {
      console.log('but this never gets logged after changes are made');
      console.log(defaultProps);
    });
  }
}

defaultProps.js looks like this:

module.exports = {
  what: 'in the world'
};

And I have the following devDependencies:

    "webpack": "^1.12.12",
    "webpack-dev-middleware": "^1.5.1",
    "webpack-hot-middleware": "^2.6.4",

I've also upgraded Node to 5.5.0, tried webpack 2, reinstalled all of the modules, but with webpack 2 the hot middleware hangs at the webpack.hot.check call, so I've reverted back to webpack 1 for now.

Can't for the life of me figure out what the problem is. :(

@ghost
Copy link
Author

ghost commented Jan 26, 2016

Hmm I've finally gotten it to detect the reloaded dependency. Apparently module.hot.accept can't be in within defaultProps.js (or any of its dependencies) for it to work in renderApp.js? It would be nice for it to continue bubbling up.

So I guess now I have to figure out where the real issue is in the actual app...

@ghost
Copy link
Author

ghost commented Jan 26, 2016

Yeah the hot reloading doesn't work for nested dependencies. It only works if I explicitly reference each dependency like so:

    module.hot.accept(['./defaultProps', './providers/index', './providers/entries'], () => {
      defaultProps = require('./defaultProps').default;

      renderApp({
        ...appInstance.props,
        ...defaultProps,
        //...appInstance.getProvidedState()
      });
    });

I was hoping I would at least be able to programmatically create the array passed to module.hot.accept (like I mentioned above), but this doesn't seem to work either:

if (module.hot) {
  let hotModules = ['./defaultProps', './providers/index'].concat(
    Object.keys(defaultProps.providers).map(key => (`./providers/${key}`))
  );

  module.hot.accept(hotModules, renderRoot);
}

@sokra if there's anything I can do to help out and make all of this work more easily, please let me know.

@ghost ghost closed this as completed Feb 5, 2016
@gaearon
Copy link
Contributor

gaearon commented Feb 29, 2016

Yeah the hot reloading doesn't work for nested dependencies.

It certainly works (normally), here is an example relying on that: reduxjs/redux#1455. You can edit any component and updates will bubble up.

@ghost
Copy link
Author

ghost commented Feb 29, 2016

Yeah I'm not sure what changed but it started working as expected out of nowhere. I think maybe it had something to do with having to map out the dependency array manually.

@ghost
Copy link
Author

ghost commented Jun 25, 2016

Finally figured out what the issue was here. If you care enough @sokra, I can explain it when I get a chance. IMO it doesn't really make any sense why it occurs and I would consider it to be a bug.

@sokra
Copy link
Member

sokra commented Jun 25, 2016

If you care enough @sokra, I can explain it when I get a chance. IMO it doesn't really make any sense why it occurs and I would consider it to be a bug.

yep, I care...

I think maybe it had something to do with having to map out the dependency array manually.

You can't pass a expression to module.hot.accept. It must be an const array of strings.

@ghost
Copy link
Author

ghost commented Jun 25, 2016

I'll try to explain it real quick and if it requires a full example to repro, I'll have to do that later.

Long story short, I expected hot reloading to work with the following 2 files configured like this, but it doesn't detect when something changes in ./components/index:

// src/App.js
// I don't care about hot reloading this file, but this file breaks the hot reloading in the next one.
// There is no circular dependency, just the components index being imported in 2 different files,
// where one file is imported by the other, but there is no circular dependency.

import { a, b, c } from './components/index';

const App = () => (
  // using a, b, c here
);

export default App;
//src/renderApp.js

import App from './App';  // this breaks the `module.hot.accept` call; the file change is never detected

if (process.env.NODE_ENV !== 'production') {
  if (module.hot) {
    const reloadFunctions = require('./utils').reloadFunctions;  // this is essentially middleware throughout the app which has full control over a, b, c
    let oldFunctions = require('./components/index');

    module.hot.accept('./components/index', () => {
      const newFunctions = require('./components/index');

      reloadFunctions(oldFunctions, newFunctions);
      oldFunctions = newFunctions;
    });
  }
}

Hot reloading works when everything is changed to this though:

// src/App.js

import * as components from './components/index';

const { a, b, c } =  components;

const App = () => (
  // using a, b, c here
);

export default App;
export { components };
//src/renderApp.js

import App from './App';

if (process.env.NODE_ENV !== 'production') {
  if (module.hot) {
    const reloadFunctions = require('./utils').reloadFunctions;
    let oldFunctions = require('./App').components;

    module.hot.accept('./App', () => {
      const newFunctions = require('./App').components;

      reloadFunctions(oldFunctions, newFunctions);
      oldFunctions = newFunctions;
    });
  }
}

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants