HMR Hooks (Discussion)

[Axelen123]

New PR here: #3148

This PR adds hooks needed for HMR support and should fix #2377. $capture_state() still returns bindings and handlers but injecting the state still works. I haven't made any tests for the new functions but I have modified the expected result of some existing tests. Here is a Webpack Demo.

If in the demo you get an error like:

ERROR in ./src/App.svelte
Module build failed (from ./node_modules/svelte-loader/index.js):
Error: Cannot find module 'css-tree/lib/parser/index.js'

Then you should run:

git clone "https://github.com/Axelen123/svelte.git#hmr"
cd svelte
npm install
npm link
cd ..
npm link svelte

[ekhaled]

This actually looks very good.
Only things I'd add are:

• Ensure those methods are only available in dev mode
• Add $$self.$recompute which basically calls $$self.$$.update if available
• Ensure capture and inject also take into account non-exported props

[Axelen123]

• Ensure those methods are only available in dev mode

Added test for that

[ekhaled]

$capture_state() still returns bindings and handlers but injecting the state still works

I think modify .map(prop => ......) in both methods to .filter(prop => prop.writable).map(prop => ...)

[ekhaled]

@Conduitry @Rich-Harris is there any way of identifying whether a prop is a method or not?

for example hello here?

<script>
export function hello(){//something}
</script>

It would be good to know component methods so that we can proxy them.

[Axelen123]

Ensure capture and inject also take into account non-exported props

It appears to take non-exported props into account except for consts sometimes. Maybe it is because we filter non-writable props?

[ekhaled]

except for consts sometimes.

That is fine I think.
If you try to mutate consts.. you do get an error in the browser
Uncaught TypeError: Assignment to constant variable.

So it makes sense if we don't try to mutate them here

[Axelen123]

Yes. It's just that if for example you have a const array that you used array.push on then all changes to that array would be reset during HMR. Code:

<script>
  export let name
  let data = "foo";
  const myarray = [];
</script>

<style>
  h1 {
    color: purple;
  }
</style>
<input bind:value={data}>
<button on:click={() => myarray.push(data)}>push</button>
<button on:click={() => console.log(myarray)}>log</button>
<p>data: {data}</p>
<h1>Hello {name}!</h1>

But it isn't too big of a problem because you should really be using the spread syntax instead.

[Axelen123]

• Add $$self.$recompute which basically calls $$self.$$.update if available

If it only calls $$self.$$.update then can't you just call $$self.$$.update?
The code already calls $$invalidate when you inject state

[ekhaled]

hmm, we might not need it.

I was asking because $$self.$$.update may or may not be in the generated code.
so calling it might result in an error.

Whereas, $$self.$recompute could be called blindly, and it would call update when available, or just result in a no op.

On a different note.... do you also use svelte-native?

[Axelen123]

On a different note.... do you also use svelte-native?

I have used it. My svelte-loader fork does have a branch for hmr with svelte-native. I could add support for svelte-native and make a demo.

[ekhaled]

Managed to get your forks up and running.
Had to also npm link svelte in svelte-loader, to get rid of the css-tree error.

This seems to be working really well.

Can we merge those branches and support both native as well as DOM?

I'm just adding a list of things we need to check (just for my benefit):

• Computed props get updated
• prop bindings work
• Event forwarding works after a hot-update
• Does it work when dom positions change in keyed lists
• can proxy code be simplified if svelte stored target and anchor in $$
• insertionPoint related code seem a bit brittle, maybe that can be improved with help from compiler
• can we get it done without having to touch $$.fragment directly

Also cc. @rixo

[Axelen123]

Actually I compared the branches and I think the regular hmr branch should work fine for svelte-native.

[Axelen123]

I made a demo here but I haven't been able to get it working on my computer becuase of some module resolution error that I am also getting with rixo's demo.
I don't know if it works but you can try it if you want.

[rixo]

Oh, you're doing something with this? Awesome :)

I've merged your hmr branches of svelte-loader and svelte-dev-helper into hmr-inject of mines.

I've updated the proxy a bit to better support your addition. I've also pushed a fix for extraneous full reloads when a child is modified after its parent so you should take it too.

One thing: $capture_state / $inject_state should always be available if they're available at all (in dev / HMR context, that is). Even if they do nothing. The proxy would find itself in an inextricable situation if it got a captured state by the previous version of the component, and the next version does not have an $inject_state function.

Also, my expectation was that $capture_state / $inject_state would essentially be drop-in replacements for my captureState / restoreState functions. That is, they would also handle listeners, and bindings, and every such details that should remain internal to svelte's compiler. I've tested slots, they work great with your captured state. I'm not sure what is still missing...

Actually my code could really use some tests because I'm already starting to lose track of what has been fixed and why. I don't know what would be a proper setup for testing webpack HMR though... Maybe some other project that does something similar has found a solution? Any idea?

Can we merge those branches and support both native as well as DOM?

Eventually.

I've had an issue that svelte-native HMR needs to require things from the nativescript package that won't be available in non native project (and webpack insist on resolving everything at compile time, even if the require are never actually executed). That's why I ended up adding a dedicated entry point for native.

But mainly, I've kept the branches separate because I wanted to be able to work on one without having to update the other immediately... And still keeping the demos working at any time, in case somebody wanted to give them a try. You know, so they are not greeted with an error if possible.

The native branch and demo have been left a little behind, I think. But I've just checked the last version of the demo I had pushed and it still works for me. At least, it runs on android and do basic HMR.

But there's still some way to go... So I don't think now is a good time to merge yet. The problem will solve itself when the code is integrated into packages with proper npm versions.

can we get it done without having to touch $$.fragment directly

Well, we're using it to hook into mount & dismount. So we would need public (dev) hooks for that. The current implementation also need to access the target / anchor to feed them to new versions of components that come over HMR. So we'd need some dev peepholes into that too.

But I'm wondering if this kind of hook / privileged access is the best approach we can come with. All these knobs and handles will have a maintenance cost for svelte. Worst case scenario, it might end up exposing some private parts that will later restrain it from evolving as it sees fit. Tying its own hands for the future. The dev API on svelte's side will still be coupled to the other side's logic, but svelte will have reduced visibility and no control over this logic's implementation...

If that's just for HMR (a big if), it might be more economical to internalize the whole component recycling concern inside svelte, behind a single piece of API with a clear simple contract instead of multiple partial ad hoc services with intricate requirements. I'm thinking something like MyComponent.$replace(otherComponentInstance). It would manage component lifecycle (destroy...), render in place, and transfer as much state as possible to the new component. And also ensure that the object reference in memory remains the same (i.e. taking the role of our current proxy).

It might seem a lot of responsibility for svelte to adopt, but it might actually make things easier for everybody. Svelte would only have to maintain a single piece of API with a clear contract to the outside world, instead of maintaining multiple little individual contracts out of the context where they are used (and it's easy to break things when you lose track of the context). In exchange of this responsibility, it wouldn't have to expose any of its internal parts, thus preserving its whole capacity to evolve. It will also control the trade off between shiny HMR and compiler evolution at any given time. For example, they could decide on their own to disable HMR for some time to make way for a big change. Existing bundlers (webpack loader...) wouldn't be impacted beyond the lost of HMR capabilities. They would keep bundling. And reloading. And in bundler plugins, HMR would be dumb to implement (save for the part of interfacing with the bundler's HMR mechanism itself). So they could be aplenty. And solid.

Also, the implementation of the recycling logic would sit inside the component's code, so it could technically see and touch the most private parts of components. So for one, the implementation should be easier from inside svelte... But, above all, it could potentially be extremely powerful. It could theoretically develop god power. The day svelte decides it's time to go serious on HMR, it could go crazy with compiler voodoo to instrument everything from component's innards to the whole component tree, and even down to catching errors at the level of individual event handlers, component initialization, etc. etc. Svelte could deliver the holy Grail of HMR.

Granted, that will (may?) only be possible if svelte manages, over time, to keep its overall complexity and public API low. Complexity compounds very fast when dealing with capricious things like HMR. Svelte3 tiny public API has been an essential ingredient of my HMR code, for example.

Well, that's just talking for now. Heck, I'm not even sure the proposed solution would work for svelte-native HMR. Need to look into that. But I think this is something that is worth addressing now. Is it really wise to start stabilizing HMR serving API already, by peppering svelte's shell with genie holes (that you can't close afterward) in the process? Shouldn't we wait for a better time, when the core team is more available, to really cash in on the power of the compiler with one bold move?

A possible alternative could be to first focus on shipping the hackish HMR for Svelte3, and accept to pay the maintenance price on our side. This way we could deliver much awaited HMR faster. We wouldn't have to wait for svelte to handle its other priorities. We wouldn't draw from svelte's forces at a time it doesn't need it. And we wouldn't force it into hasty decisions it might regret later.

Our hackish HMR quality should be able to approximate what can be found in other frameworks with big promises (that they struggle to deliver on consistently because of their own complexity). Then it can cut down some limitations by moving inside the framework compiler. Then it can improve. And improve. Until it sets new standards for everybody else. Hugh.

Please excuse the monologue. What are your thoughts?

[ekhaled]

That is, they would also handle listeners , and bindings , and every such details that should remain internal to svelte's compiler.

do you need access to listeners and bindings? for native?

[ekhaled]

@rixo @Axelen123
Since svelte-native HMR is not something that would be available without some sort of jiggery-pokery....
I suggest that you minimise the diff on svelte-loader/index.js.
It would be easier to get the PR accepted if the review surface area is minimal.

---

The current implementation also need to access the target / anchor to feed them to new versions of components that come over HMR. So we'd need some dev peepholes into that too.

I was suggesting we put target and anchor in $$, so we can pull them out later.
Also a __mounted flag, that will tell us if component has been mounted.
That would make the maintenance burden on svelte quite minimal.
Something like this in runtime/internal/Component.ts:

export function mount_component(component, target, anchor) {
	const { fragment, on_mount, on_destroy, after_render } = component.$$;

+	component.$$.target = target;
+	component.$$.anchor = anchor;
+
	fragment.m(target, anchor);

+	component.$$.__mounted = true;
+

[Axelen123]

The current implementation also need to access the target / anchor to feed them to new versions of components that come over HMR. So we'd need some dev peepholes into that too.

I was suggesting we put target and anchor in $$, so we can pull them out later.
Also a __mounted flag, that will tell us if component has been mounted.
That would make the maintenance burden on svelte quite minimal.
Something like this in runtime/internal/Component.ts:

export function mount_component(component, target, anchor) {
	const { fragment, on_mount, on_destroy, after_render } = component.$$;

+	component.$$.target = target;
+	component.$$.anchor = anchor;
+
	fragment.m(target, anchor);

+	component.$$.__mounted = true;
+

I could add that if you want :)

[ekhaled]

Sure, you will need to update a few lines down in function init() too.

But lets first make sure if we can simplify proxy code if those are available, @rixo ?
We don't wanna be adding to svelte api surface unless it helps our cause.

[Axelen123]

If that's just for HMR (a big if), it might be more economical to internalize the whole component recycling concern inside svelte, behind a single piece of API with a clear simple contract instead of multiple partial ad hoc services with intricate requirements. I'm thinking something like ?MyComponent.$replace(otherComponentInstance). It would manage component lifecycle (destroy...), render in place, and transfer as much state as possible to the new component. And also ensure that the object reference in memory remains the same (i.e. taking the role of our current proxy).

So should we replace $capture_state and $inject_state with $replace?
I got the idea here.

I think we should introduce new methods that are explicitly designed for this purpose — e.g. component.$capture_state() and component.$inject_state(captured_state). $inject_state would work basically the same way $set does currently, except that it would inject any local writable vars, not just exported ones.

[rixo]

do you need access to listeners and bindings? for native?

That's not especially for native. They're not in the props. If we don't pass them ourselves to a new version of a component we're creating, them bound variables & listeners registered by the parent are lost on HMR update.

For example, in this:

<script>
import Child from './Child.svelte'
let bound
const onEvent = () => console.log('called')
</script>

<Child bind:value={bound} on:myevent={onEvent} />

The first time Child is modified & HMR updated, the bound value and the event will stop functioning. It works when the parent is updated, because Svelte passes them again to the Child, but not if we're just recreating the component in the proxy.

I'd say they are part of the component state, as far as I'm concerned.

[rixo]

So should we replace $capture_state and $inject_state with $replace?

My intent was just to discuss the idea. Got a little bit carried away while describing it :-x... To be clear, I'm not even sure it is a good idea. But maybe it is. I would be happy to ear other opinions on it.

[rixo]

@Axelen123 And $replace would be an effort of another magnitude... Basically, all the logic of the proxy should be integrated into svelte. And you wouldn't be able to reuse much (if any) of the existing proxy implementation. Most of the current proxy is mainly to interface from the outside.

But I still think it could possibly open great opportunities for svelte HMR. So, if you are able and willing to do it, feel free. I'd be grateful. Even if it does not work in the end ;)

[rixo]

I was suggesting we put target and anchor in $$, so we can pull them out later.
Also a __mounted flag, that will tell us if component has been mounted.

But lets first make sure if we can simplify proxy code if those are available, @rixo ?

Maybe a little disclaimer is due first. At this point, I'm not even sure the current proxy implementation is complete (I mean it can handle all the cases Svelte will throw at it, I'm not talking about luxury features). I suppose it is not. I wonder if I have missed something big.

The only feedback I've got so far is from my own usage. And if there's a Svelte feature I don't know, I will have missed it while coding the proxy, and never used it afterward to realize my mistake...

My biggest interrogation ATM is what will happen if a component's DOM nodes are moved after its creation. Does Svelte support that? The current proxy assume that a component will be mounted when it is created, or just after, and never move afterward. This might need revision. I would love some inputs on this.

That being said, I don't think that would be enough.

Anchor & target are only really interesting at mount time, where they can be used to determine the insertion point. After that, other components may push the anchor element away, so it isn't reliable anymore.

What would help greatly would be if svelte itself could tell us the insertion point. That would need to be a function (or a getter, but something dynamic). But that seems very hard to know, even from svelte's internals, before the component is actually mounted. And then the information seems hard to track, except by putting a dummy element in the DOM like we do.

If svelte gives us the insertion point, then we can get rid of the hackish hook on $$.fragment.m, and of essentially all the code in lib/proxy-adapter-dom.js. If we can't have the insertion point, then we'll need at least a mount callback that gives use anchor & target.

We'll still need a solution for the hook on $$.fragment.d.

Public onMount and onDestroy might not be enough for us because they have to be called from the component's constructor. Maybe with good timing we can trick svelte into thinking our component is still its "current component", but I wouldn't call that moving away from internals. And anyway onMount doesn't give us anchor & target.

If we don't have a mount callback (because svelte gives us the insertion point), then we might need __mounted. Like I said, this is not currently handled by the proxy, so it wouldn't be needed by the current code. But I suppose we should probably handle the case (namely of a component that is destroyed before it has been mounted), at least for peace of mind. Should we?

Then we need to rewrite the code that proxies everything under $$ (outside world -> proxy -> current component instance). If we don't need to interfere with it for our callback needs, then we might be able to do that by blindly deeply copying everything under $$ on the proxy when we instantiate a proxied component (or an actual ES Proxy, but I guess this is not an option). But this is yet to be confirmed in practice.

Also, this is assuming $inject_state will rectify $$.callbacks and $$.bound. And maybe $$.context (but in my usage, the context API seems to work without the proxy doing anything special about it). And also $$.props that it already supports. And every such thing that will come in the future, or any such thing that I would have missed in the present.

With all that, we should be able to get rid of all references to private things under $$ (we'd still need to know about $$ itself but that's probably OK). I'm not sure, but I think this should also get rid of any implicit dependency on knowledge about svelte component's internal functioning.

[rixo]

Oh, and I just noticed... IMO $capture_state and $inject_state should probably not be put directly on the component, to avoid naming clashes (it's unlikely, but it's not documented that users are not allowed to use those names either). They should probably go under $$ (as $$.capture_state for consistency). And we'd get to know about them. (Isn't it ironic?)

[Conduitry]

I haven't been following this discussion, but I think $capture_state and $inject_state directly on the component are the right place, not down in the $$ object. Users are already forbidden (by the compiler) from having props or methods that begin with $, and we are using that prefix for the few methods that already come on all components ($set, $on, $destroy). We want the HMR hooks to be part of the stable public API, and the things in $$ are internal and subject to change at any time.

[rixo]

Ah yes, there are indeed public. Silly me.

@Conduitry Can you tell us if we should worry about unmounted components? That is, can a component be created, not mounted, and not destroyed? (I've seen that it happens very briefly for child components, but are there other cases?)

More generally, can a component ever be unmounted (without being destroyed) in svelte3? If not, do you think it could change in the future?

[rixo]

I think I have panicked a little bit here, and made things are to read...

@Axelen123 I'm affraid you may have to recreate this PR for the discussion to be intelligible to whose who will need to review it. I'm sorry for derailing your effort.

@ekhaled We should probably start with the beginning and discuss what are the plans regarding the experimental code I've shown you, in the corresponding issue.

[Axelen123]

New PR here: #3148