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

Issue with Webpack HMR (hot module reloading) #564

Closed
vuau opened this issue May 24, 2016 · 12 comments · Fixed by #664
Closed

Issue with Webpack HMR (hot module reloading) #564

vuau opened this issue May 24, 2016 · 12 comments · Fixed by #664

Comments

@vuau
Copy link

vuau commented May 24, 2016

Is there anyone here using webpack Hot Module Replacement successfully with skatejs components? I currently got this issue.

[HMR] Cannot apply update. Need to do a full reload!
[HMR] Error: Failed to execute 'registerElement' on 'Document': Registration failed for type 'tiny-button'. A type with that name is already registered.
@treshugart
Copy link
Member

We are using it. Can you give us some more info?

  • Skate version
  • Webpack config
  • Webpack version
  • Webpack dev server version

Error: Failed to execute 'registerElement' on 'Document': Registration failed for type 'tiny-button'. A type with that name is already registered.

This happens when you try and register the same component twice. I'm thinking this error might be causing the problem where the update can't be applied. Could it be possible that you're executing the tiny-button component registration twice?

@vuau
Copy link
Author

vuau commented May 24, 2016

this is my current setting

package.json

{
...
    skatejs": "^0.15.3",
    "webpack": "^1.13.1",
    "webpack-dev-server": "^1.14.1"
...
}

webpack.config.js

require('core-js');

const isDebug = process.env.NODE_ENV !== 'production';
const PORT = 8000;

var webpack = require('webpack');
var HtmlWebpackPlugin = require('html-webpack-plugin');
var HTMLWebpackPluginConfig = new HtmlWebpackPlugin({
  template: './src/index.html',
  filename: 'index.html',
  inject: 'body'
});
var ExtractTextPlugin = require('extract-text-webpack-plugin');
var scssLoader = {
  test: /\.scss$/
};
var fontLoader = {
  test: /\.woff2|eot|ttf|svg|woff?(\?v=[0-9]\.[0-9]\.[0-9])?$/,
  loader: 'url'
};

var config = {
  entry: [
    './src/js/main.js'
  ],
  module: {
    loaders: [{
      test: /\.js[x]?$/,
      loaders: ['babel-loader?cacheDirectory&presets[]=es2015'],
      exclude: /(node_modules)/
    }]
  },
  debug: isDebug,
  devtool: isDebug ? 'source-map' : false,
  output: {
    path: './dist',
    filename: 'bundle.js',
    sourceMapFilename: '[file].map'
  },
  plugins: [
    HTMLWebpackPluginConfig,
    new webpack.DefinePlugin({
      'process.env.NODE_ENV': isDebug ? '"development"' : '"production"',
      __DEV__: isDebug,
    }),
    new webpack.ProvidePlugin({
      "window.MutationObserver": "mutationobserver-shim"
    })
  ],
  devServer: {
    contentBase: './dist',
    inline: true,
    watch: true,
    hot: true,
    port: PORT
  }
};

if (isDebug) {
  config.entry.push('webpack/hot/dev-server');
  config.plugins.push(new webpack.HotModuleReplacementPlugin());
  scssLoader.loader = 'style!css?sourceMap!sass?sourceMap';
} else {
  config.plugins.push(new webpack.optimize.CommonsChunkPlugin('common.js'));
  config.plugins.push(new webpack.optimize.DedupePlugin());
  config.plugins.push(new webpack.optimize.UglifyJsPlugin({
    compress: {
      warnings: true
    }
  }));
  config.plugins.push(new webpack.optimize.AggressiveMergingPlugin());
  config.plugins.push(new ExtractTextPlugin('bundle.css'));
  scssLoader.loader = ExtractTextPlugin.extract('css-loader?sourceMap!sass-loader?sourceMap=true&sourceMapContents=true');
}

config.module.loaders.push(scssLoader);
config.module.loaders.push(fontLoader);

module.exports = config;

@vuau
Copy link
Author

vuau commented May 25, 2016

@treshugart you was right. When webpack tried to update the module 'tiny-button', it went through the constructor skate('tiny-button', ...) then registered the component again.
Am I using it wrongly? Could you share the way you are using HMR in Skatejs components?

@treshugart
Copy link
Member

We just started a new project and it turns out we haven't enabled it yet, so I was wrong. That said, the issue will probably be because the browser keeps an internal registry of all custom elements that have been registered. When the module reloads it causes it to try and register again.

What I suggest trying is exporting your Skate definition without registering the component and have a separate file import them and register them. I'm not sure if this will work, though, as I haven't tried it.

Unfortunately this is a limitation of custom elements as you can't deregister them. One possible solution is to try and update the definitions in the registry, but I'm not sure I like the idea of them being mutable especially when it's not in the plans of the spec to allow such behaviour. Thoughts?

@treshugart
Copy link
Member

treshugart commented May 26, 2016

I can confirm this is an issue and we want to try and find a way for it to work.

There's some challenges, though. Even if we can update definitions, it's hard to know exactly how to update components on the page. For example, if you remove the created callback from your component, how is the component supposed to update? This raises more questions:

  • Should it just be replaced with a new instance of the component?
  • How do we transfer the old state to the new state?
  • You can try and use state() to do so (slated for 1.0+), but what if the old state is incompatible with the new state?

Given the Custom Element V1 spec doesn't have a way to re-register modules, the first step in solving this would be to figure out if there's even a workaround for that. From there, we'd have to answer the questions above.

For now, though, to get full updates, you'll have to use standard reloading instead of HMR.

@treshugart
Copy link
Member

One solution might be to mutate the existing definition. Something like:

const OldCtor = window.customElements.get(name);
const NewCtor = Ctor;
if (OldCtor) {
  Object.assign(OldCtor, getOwnPropertyDescriptors(NewCtor));
  return OldCtor;
}

Notes:

  • getOwnPropertyDescriptors() probably won't work for inheritance; we'd need to walk the prototype chain and get all descriptors
  • We support v0 custom elements so window.customElements.get() won't work for v0; we'd need to store our own registry internally for this
  • We need to find out what the expected behaviour is for components already in the DOM

I spoke with @lukebatchelor about this IRL and he's going to PoC it in our stack and try to find a working solution for it. We'll keep this issue updated.

@treshugart
Copy link
Member

@lukebatchelor can you provide details in how we were to get around this temporarily?

@lukebatchelor
Copy link
Contributor

Temporarily we have separated out the component definition and the actual call to define.

const definition = {
  render(elem) {
    /* ...   ... */
  },
  props: {
    /* ... ... */
  },
};


/* The constructor for our component */
export default () => define('my-component', definition);

export { definition };

So that we can import the function to register OR import the definition itself.

To use HMR we import the definition and when we register the component, we append a random string to the end of it to make sure it is unique.

I will try to get around to taking a look at the above solution today though and see how we go.

@treshugart
Copy link
Member

Addressing in #663.

treshugart added a commit that referenced this issue Jul 13, 2016
…name.

If a component with the same name has already been registered, Skate will choose a different name
and use the unique name.

#564
treshugart added a commit that referenced this issue Jul 13, 2016
#564, #663 - Multiple components with the same name
@treshugart
Copy link
Member

@treshugart treshugart changed the title Issue with webpack HRM Issue with Webpack HMR (hot module reloading) Jul 14, 2016
@Toilal
Copy link

Toilal commented Mar 23, 2017

Any recipe to enable Hot Module Replacement in a SkateJS webpack based project ?

@treshugart
Copy link
Member

@Toilal have a look at the define() docs. It's worth reading the whole thing, but there's a small section at the end describing the pitfalls of using customElements.define() with HMR.

I think in order to accomplish this you'll have to use define() without specifying a tag name, at least in HMR projects.

If you're building an app, not defining tag names is doable because you'll probably be writing JSX and / or just passing around constructors, as opposed to string tag names. However, it's harder when you're building sharable components as you want to make the available in HTML, which means defining a name.

To get around this if you're building components, is to export the constructor from your component and let the consumer manually define them. In your tests, you can define them using Skate's define for HMR while still allowing the consumer to choose the name.

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

Successfully merging a pull request may close this issue.

4 participants