Nodes not focusable anymore after hot reload

[ChristiaanScheermeijer]

Describe the bug
In a Vite.js live reload environment, focusable elements are not focusable anymore after hot reloading the component.

To Reproduce
Steps to reproduce the behavior:

1. Init a simple Vite.js project
2. Add the Norigin Spatial Navigation library
3. Add a simple component that uses useFocusable
4. Run the project
5. Make a change to the component

Expected behavior
After a hot reload, the component should still be focusable.

Additional context
Not sure if it is Vite.js specific. Perhaps it can be reproduced in Webpack as well.

I've cloned and linked the repo to do some debugging and found that the useEffectOnce is the culprit. After a hot reload, the useEffect is triggered which removes the node from the registry. But the effectCalled ref isn't cleared, so the node never gets added again.

[predikament]

Hello @ChristiaanScheermeijer, thanks for opening an issue.

We are not actively using Vite ourselves, but are sorry to hear there's such an issue happening.

I've cloned and linked the repo to do some debugging and found that the useEffectOnce is the culprit. After a hot reload, the useEffect is triggered which removes the node from the registry. But the effectCalled ref isn't cleared, so the node never gets added again.

If you can think of a solution that would work without having any adverse effects then you could perhaps open a PR?

Otherwise it might take a bit for us to get around to looking into this issue, although we will hopefully resolve it eventually.

I will update this issue once we have some more concrete feedback to give.

Cheers,
@predikament

[ChristiaanScheermeijer]

Hi @predikament,

Thanks for looking into this issue. I couldn't reproduce this issue using Webpack, so I've migrated the example application to a Vite.js application here: https://github.com/ChristiaanScheermeijer/spatial-navigation-vite-example

To reproduce the bug, change the menuItemText to something different and wait for a hot reload.

Before I can fix this issue, can you explain which problem the useEffectOnce solves exactly in what scenario?

[predikament]

Hello again @ChristiaanScheermeijer,

The useEffectOnce hook was added as part of this release: https://github.com/NoriginMedia/Norigin-Spatial-Navigation/releases/tag/v1.1.0 - It's used due to the effects running twice when using React 18 (in Strict mode), basically.

A bit strange that this would cause this side-effect when using Vite but not Webpack.. I assume somewhere in their differences lies a potential solution.

If you don't figure it out then I will try to have a look once I have a bit of time to dedicate to it.

Thanks!

[predikament]

Hello @ChristiaanScheermeijer!

I believe I have figured out your issue and I do not think it is at all related to our library actually -
It seems more like an issue related to how you have written your code, in combination with how React + Vite HMR works.

When checking your project (Thanks again for sending that btw!) I noticed that your <Text> component is written as an anonymous function:

import React from 'react';

export default function() { // <-- Unnamed
  const menuItemText = 'change me!';

  return <div>{menuItemText}</div>;
};

So I ran your project and changed your component to this:

import React from 'react';

const Text = () => {
  const menuItemText = 'change me!';

  return <div>{menuItemText}</div>;
}

export default Text;

And now everything is still focusable and works as intended whenever I do any hot-reloading..
So I think it's more an issue here that there's something with Vite where it does not reload it due to this having been unnamed.
(All components are modules when it comes to HMR, so I guess it doesn't know that it should reload something that it does not have a named reference to? No idea tbh, just theorizing.. 🤷)

Regardless of this I would recommend naming your component's export functions, so to have a better / more easily understandable stack-trace when debugging.

Let me know if this is enough feedback for you to close this issue as resolved. Thanks!

Cheers,
@predikament

---

Edit: I did some quick searching and found this: vitejs/vite#4298 (comment)
So this is a known issue behavior for React+Vite's HMR when using unnamed functions, unfortunately. ☺️

[ChristiaanScheermeijer]

Hi @predikament,

Thank you for the extensive update! That's a really good find, I hope it didn't cause you too much trouble getting here ✌️

I almost feel bad for saying this, but the project where we found the issue, doesn't contain any unnamed components.

But the HMR topic is interesting... perhaps it is something similar causing this issue due to a different problem.

[predikament]

I almost feel bad for saying this, but the project where we found the issue, doesn't contain any unnamed components.

Ah, alright. No worries at all - It was interesting to investigate a bit regardless. 🕵️

But the HMR topic is interesting... perhaps it is something similar causing this issue due to a different problem.

I would strongly assume that your actual problem is somehow a by-product of this, yes.

There is a PR that was recently merged to Vite main with some reworks to its HMR refresh logic here: vitejs/vite#10239

Perhaps you can check out the latest version of Vite sometime to see if it has resolved the issue you see in your project?
Outside of this I am unfortunately out of solutions.

Knowing this behavior with the Vite HMR I don't feel comfortable trying to fix this in our project right away, at least not until that HMR change has actually been released to see if that would resolve it, but we will keep it in mind going forward.

Best of luck to you and happy coding! 😸