[plugin-svelte] Fast Refresh doesn't recognize variable value changes via HMR

[melissamcewen]

As mentioned in discussion #1536 . To replicate use the latest CSA app-template-svelte. Then add this code to your app.svelte

<script>
    let name = 'world';
</script>

<h1>Hello {name}!</h1>

Start up the server, then try changing the value of name. The changes do not show up unless you refresh.

[melissamcewen]

This might actually be fast refresh not working, not HMR, curious to see since the terminology can be confusing haha.

[melissamcewen]

CC @UARTman @markfirmware

[markfirmware]

Thanks - I have a repro repo here:
https://github.com/markfirmware/snowpack-svelte-hmr/blob/master/README.md

I did not know there was a fast refresh distinct from hmr.

[markfirmware]

This might actually be fast refresh not working, not HMR, curious to see since the terminology can be confusing haha.

@melissamcewen @FredKSchott @stramel @UARTman - just changing the text Hello to Hi does work as desired. Does this determine whether the problem is fast refresh versus hmr?

[UARTman]

Since I don't see this brought up, I will reiterate what I said in discussion threads.

1. The problem is with variables and, probably, only variables. If you declare a function with a proper declaration and not an arrow function it will work.
2. After you change a variable the view stops responding to changes in data. If you change data inside the event handler, it will still fire, but the view won't change.

I am not sure how much help this provides but this is all I have now.

[FredKSchott]

paging HMR Guru @rixo :) I think that this is a limitation of https://github.com/rixo/svelte-hmr, the HMR library that we use internally.

@rixo anything that we can do here to help? If this isn't possible, but you can detect it, then you could call import.meta.hot.invalidate() inside of your accept handler to tell the browser to refresh. Obviously not as great as HMR, but better than silently failing to update.

https://github.com/snowpackjs/esm-hmr#invalidate

[UARTman]

It isn't variable name changes, it's variable value changes.

[FredKSchott]

Updated title! thanks for the correction

[rixo]

It's not a bug, it's a feature!

Not kidding.

This is preservation of local state in action. It's a 2-edged sword. You can turn it off with an option.

  plugins: [
    [
      '@snowpack/plugin-svelte',
      {
        hmrOptions: {
          noPreserveState: true,
        },
      },
    ],
    '@snowpack/plugin-dotenv',
  ],

Without it, though, you wouldn't have the state of the counter in the default Svelte create-snowpack-app example preserved across updates (it would reset to 0 each time):

<script>
  import {onMount} from 'svelte';
  let count = 0;
  onMount(() => {
    const interval = setInterval(() => count++, 1000);
    return () => {
      clearInterval(interval);
    };
  });
</script>
<p>Page has been open for <code>{count}</code> seconds.</p>

If this counter was a flag to have a dialog open, for example, on each HMR update the flag would reset and you would have to re-click the open button to see your dialog. With this option, the "in-memory" state is preserved, and the modal stays open.

On the other hand, like this issue shows, this can be confusing and it can also get in the way sometimes. Without it, you still benefit from having the state of all the rest of the app -- everything above and around the impacted component(s) -- preserved / unaffected.

This is really a question of taste and there is no clear winner, so I'm never satisfied with what should be the default for this option. Some other plugins have defaulted it to false...

There is normally a way to disable it temporarily by putting a @!hmr flag in your code but I've just realized it was not working with Snowpack. This will get fixed.

Also, in the discussion leading to this issue, the event handler should probably be a const. If it were, Svelte wouldn't track its changes, and its state wouldn't be preserved by HMR.

<script>
  let counter = 5;

  const handle = () => {
    counter = counter + 1;
  };
</script>

Counter:
{counter}

<button on:click={handle}>+1</button>

One shouldn't use let indiscriminately ;)

[melissamcewen]

Ohhh I thought maybe it was something like this, thanks for the detailed explanation

[rixo]

Fixed @!hmr in svelte-hmr@0.11.6. It wasn't specific to Snowpack in fact.

[FredKSchott]

oh interesting! TIL!

So just to be clear, this behavior only exists for let and var defined variables, but not const.

Thinking out loud: I wonder if there would be some way to keep track of the "original" value of each of these, and then update if a new value was set. Something like:

original: let foo = 1;
update 1: let foo = 1; // not changed, do nothing
update 2: let foo = 1; // not changed, do nothing
update 3: let foo = 1; // not changed, do nothing
update 4: let foo = 42; // changed! update in browser, overwriting browser value

[rixo]

I'm going to disable preservation of state since it's confusing people. This issue has popped a few times already. I don't think it's really wanted in most case anyway.

For special cases where you want it, I'll had a escape hatch with a magic comment processed by svelte-hmr. Ideally I'd like to make it preserve state per individual variable.

Something like this:

let counter = 0 // @hmr:keep
let dialogOpen = false // @hmr:keep

Another benefit of this approach is that the simple counter demo found in most HMR enabled templates can demonstrate the usage of this special comment. That's about all the HMR docs I expect most users to read...

@melissamcewen @markfirmware @UARTman Any opinion on this?

[UARTman]

I think making state preservation opt-in instead of opt-out, both globally and variable-wise is a right decision. The current model adds a layer of complexity to the development process (should this variable be updated or kept?) that is not obvious and sometimes changes the behavior in unexpected ways.

Also, do you know that changing a let variable breaks a component (instead of reloading it)?

[rixo]

Thanks.

Also, do you know that changing a let variable breaks a component (instead of reloading it)?

No? How do you mean?

[UARTman]

I wrote about it here. I'll try to replicate it again when I have time.
#1536 (comment)

[rixo]

OK that's how preservation of state is supposed to work: keep the existing "in-memory" value of the variable instead of resetting it as per the assignments in the code. Think of the counter example in the default create-snowpack-app template: it wouldn't keep its value if HMR reset it to 0 on each update...

So I wouldn't call this a "breakage" but it is arguably a confusing behaviour, and its hard to anticipate or understand with no knowledge of HMR functioning. I also do believe that HMR shouldn't require users to read docs or have any special knowledge. I want it to "just work" by default, that's why I'm changing the default to use the simpler, more predictable, behaviour. People who want to change this default will be the ones expected to have some understanding of what they're doing.

[FredKSchott]

(Moved to discussion, since this is no longer considered a bug.)

[jansphil]

Hey,

I found this thread after trying to figure out why the counter example from the website (https://www.snowpack.dev/tutorials/svelte) is no longer working and actually is being reset to 0. let count = 0; // @hmr:keep does the trick but it would be nice to include this detail in the tutorial.

Thanks for the great tool!

[nous-]

I followed the tutorial as well, and also used this example, but the count resets everytime I save a change to App.svelte. Is this a new bug? It wasn't clear we have to annotate variables with // @hmr:keep we want to retain.

[rixo]

Yes, the documentation needs to be updated. It was accurate when first written, but the default state preservation in Svelte HMR has changed.

You can get the previous behavior, that would keep the counter running without the need for a special comment, by setting the preserveState option of the plugin:

["@snowpack/plugin-svelte", {
  hmrOptions: { preserveState: true }
}]

Not entirely sure which way the documentation should be fixed, though.

[nous-]

@rixo I've tried this suggestion. Still requires using // @hmr:keep.

[rixo]

@nous- Oh, yes you're right... The option name is actually preserveLocalState, sorry.

    ['@snowpack/plugin-svelte', {
      hmrOptions: {
        preserveLocalState: true
      }
    }],

There is a typo in svelte-hmr docs that I'm going to fix.

I think the longer option name is better, because HMR always preserves a lot of app state "around" the impacted component, independently of what it does with local variables. Also, there is some internal state of the components (event listeners, bound variables...) that needs to be transferred in very case.