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

Introduce hot module replacement (reload) #739

Closed
roman-vanesyan opened this issue Jul 30, 2017 · 10 comments
Closed

Introduce hot module replacement (reload) #739

roman-vanesyan opened this issue Jul 30, 2017 · 10 comments

Comments

@roman-vanesyan
Copy link

I don't know svelte internals, and if it's possible to implement hot-reloading at all, but any way the feature will greatly help in development.

Feel free to close issue, if no way to implement it (unlikely 😃), or not interested for now :)

@PaulBGD
Copy link
Member

PaulBGD commented Jul 31, 2017

Svelte doesn't really have an external way to access "trees", but if we want any dev tools then we'll probably need some way to locate nodes.

@rob-balfre
Copy link
Contributor

Don't think so currently, It's on the roadmap for v2 . See #622

@Rich-Harris
Copy link
Member

As far as I can tell, the real problem is one of state. If I have a situation like this...

<!-- App.html -->
<Foo a='{{1}}'/>

<!-- Foo.html -->
<p>a: {{a}}</p>
<p>b: {{b}}</p>

<script>
  export default {
    data: () => ({ b: 1 }),

    oncreate() {
      let { b } = this.get();

      const interval = setInterval(() => {
        this.set({ b: ++b });
      }, 1000);

      this.on('destroy', () => clearInterval(interval));
    }
  };
</script>

...then we'll have this to begin with...

<p>a: 1</p>
<p>b: 1</p>

...but b will keep incrementing in private state belonging to Foo. So if App.html changes, we need to re-render everything, and b gets reset to 1. In other words, HMR only really works if your entire state tree is accessible at the top level — i.e. you're using something like Redux.

So I wonder if we would need to have an HMR-friendly mode where components didn't have the set method, and two-way binding was forbidden.

@ekhaled
Copy link
Contributor

ekhaled commented Dec 7, 2017

Now that we have Store support, is this something that can be looked at again?

I managed to hack together a very rough HMR setup using Store and Webpack.
But it will need support from within the compiler.

What I did was basically add this to the entry file:

if(module.hot){
  module.hot.accept();
}

and then in every component had an oncreate method that did this:

...
oncreate() {
  var self = this;
  if (module.hot) {
    module.hot.dispose(function () {
      self.destroy();
    });
  }
}
...

And just that worked very well (I haven't tried on a complex codebase yet).

But as you can see, patching every component with that snippet manually is not ideal.
And, I can't think of a way to patch it after the svelte compiler has processed the file without delving into some AST.
Maybe the compiler could add that snippet (or similar) during dev mode.

I'm not sure what the best approach is, just throwing ideas out there.

@Rich-Harris
Copy link
Member

Strongly agree. Am currently immersing myself in Webpack, and this will definitely be an area of focus in the near future.

@ekhaled
Copy link
Contributor

ekhaled commented Jan 4, 2018

svelte-hot-loader has reached version 1.

Give it a whirl. Would love some feedback on that

@arxpoetica
Copy link
Member

@Rich-Harris two questions. 1) do you plan on incorporating the awesomeness of @ekhaled's hot loader, and 2) should this ticket be moved over to https://github.com/sveltejs/sapper, since I would guess that's where it would be implemented? (or does it make sense to just leave it in a plugin?)

@ekhaled
Copy link
Contributor

ekhaled commented Jan 10, 2018

IMO hot loading should be baked into svelte-loader, rather than relying on a plugin.
I have a opened a [WIP] PR to that end sveltejs/svelte-loader#40

If @Rich-Harris thinks the PR looks good, I will try and get it finished.

@ekhaled
Copy link
Contributor

ekhaled commented Jun 29, 2018

@arxpoetica I think this can be closed now. Right?

@arxpoetica
Copy link
Member

Yup

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

No branches or pull requests

6 participants