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

State resets with React Fast Refresh #1150

Closed
alan-nf opened this issue May 10, 2022 · 31 comments
Closed

State resets with React Fast Refresh #1150

alan-nf opened this issue May 10, 2022 · 31 comments
Assignees

Comments

@alan-nf
Copy link

alan-nf commented May 10, 2022

Expected: when React Fast Refresh updates UI on save, jotai state should not be lost. E.g. a primitive counter atom like const countAtom = atom(0) should maintain it's value across fast refreshes.
Actual: all jotai global state resets when React Fast Refresh updates the UI.

Codesandbox: https://codesandbox.io/s/jotai-react-refresh-bug-807nez?file=/src/Jotai.tsx
Local demo: https://github.com/alan-nf/jotai-react-refresh-issue

Instructions:

  1. Click the +1 button to increment the counter
  2. Edit the text in the Copy component to say something else. The counter will be reset back to 0.

NOTE: related discussion about React Fast Refresh being broken in #1025. I cannot reproduce things being totally broken and I'm just seeing this reset issue.

@dai-shi
Copy link
Member

dai-shi commented May 11, 2022

@Thisen Can you take a look?

@Thisen
Copy link
Collaborator

Thisen commented May 11, 2022

I'll take a look!

@Thisen Thisen self-assigned this May 11, 2022
@Thisen
Copy link
Collaborator

Thisen commented May 11, 2022

@alan-nf, the Codesandbox doesn't seem to be using the jotai/babel/plugin-react-refresh. Am I blind? 😅

@alan-nf
Copy link
Author

alan-nf commented May 11, 2022

No you're not blind, heh. It's using Create React App which uses react refresh under the covers

from https://javascript.plainenglish.io/react-fast-refresh-the-new-react-hot-reloader-652c6645548c:

Starting from 4.x, Create React App enables React Fast Refresh by default. If your project is built off of Create React App, all you need to do is upgrade.

more specifically, it's using @pmmmwh/react-refresh-webpack-plugin@^0.5.3

This is where it's hooked up: https://github.com/facebook/create-react-app/blob/main/packages/react-scripts/config/webpack.config.js#L35

@alan-nf
Copy link
Author

alan-nf commented May 11, 2022

And, if you grab the demo that can run locally, you can save a file edit and then see the network requests related to the react refresh update:

image

@alan-nf
Copy link
Author

alan-nf commented May 11, 2022

I noticed that the hot update includes the atom declaration, presumably because it's in the same file as the component I'm editing. If I move that component to a separate file and edit it there, the jotai state stops resetting. That seems like totally reasonable behavior, and therefore isn't really reproducing the issue that caused me to file this bug report in the first place.

In my codebase, editing files that do not have the jotai atom declarations are still resulting in the reset, so I guess I'm hitting a different bug. I'll try to get a simple repro of that issue.

@alan-nf
Copy link
Author

alan-nf commented May 11, 2022

Ugh, I can't reproduce with vite, even when I perfectly match the versions of all the packages in my codebase. Here it is working just fine: https://stackblitz.com/edit/vitejs-vite-2tuyjr?file=src/App.tsx

I'm going to try to figure out why this isn't working specifically in my codebase. I'll reopen this issue if I can get a correct repro of the problem.

@dai-shi
Copy link
Member

dai-shi commented Jun 3, 2022

#1203 (comment)

@Thisen can you communicate with @alan-nf about this?

@dai-shi dai-shi reopened this Jun 3, 2022
@Thisen
Copy link
Collaborator

Thisen commented Jun 7, 2022

I'll look into this.

@alan-nf can you add an updated reproduction?

@alan-nf
Copy link
Author

alan-nf commented Jun 13, 2022

I'll work some more on getting a proper repo.

Question: if I'm using vite and the @vitejs/plugin-react vite plugin, do I need anything else to get full react refresh support? Do I still need the jotai/babel/plugin-react-refresh babel plugin?

@bstro
Copy link

bstro commented Jun 15, 2022

fwiw I'm experiencing this issue as well w/ Vite 2.9.9, React 18, Jotai 1.7.2.
Not sure if this is helpful, but here's a repro on stackblitz.

steps to reproduce:

  • click the count is 0 button a few times
  • add some text to the document.

@Thisen
Copy link
Collaborator

Thisen commented Jun 20, 2022

I'll work some more on getting a proper repo.

Question: if I'm using vite and the @vitejs/plugin-react vite plugin, do I need anything else to get full react refresh support? Do I still need the jotai/babel/plugin-react-refresh babel plugin?

Yes, we only support React Refresh with the jotai/babel/plugin-react-refresh.

@Thisen
Copy link
Collaborator

Thisen commented Jun 20, 2022

fwiw I'm experiencing this issue as well w/ Vite 2.9.9, React 18, Jotai 1.7.2. Not sure if this is helpful, but here's a repro on stackblitz.

steps to reproduce:

  • click the count is 0 button a few times
  • add some text to the document.

I'll look into vite support.

@Thisen
Copy link
Collaborator

Thisen commented Jun 21, 2022

@bstro have you tried to configure vite to use the Jotai Babel preset?
https://github.com/vitejs/vite/tree/main/packages/plugin-react#babel-configuration

@bstro
Copy link

bstro commented Jun 22, 2022

@Thisen yes, I copied the setup from the guide on my project and it has no effect.

I can't use it in the StackBlitz example—for some reason adding the jotai babel plugin triggers this compilation error: [plugin:vite:react-babel] /home/projects/vitejs-vite-akwqmd/src/main.tsx: _babel_template.default is not a function

@Thisen
Copy link
Collaborator

Thisen commented Jun 23, 2022

@Thisen yes, I copied the setup from the guide on my project and it has no effect.

I can't use it in the StackBlitz example—for some reason adding the jotai babel plugin triggers this compilation error: [plugin:vite:react-babel] /home/projects/vitejs-vite-akwqmd/src/main.tsx: _babel_template.default is not a function

Can you make a reproduction in Github repo that I can clone?

@bstro
Copy link

bstro commented Jun 26, 2022

Here's a reproduction on stackblitz of my state reset issue.

The bug I'm seeing might be related to jotai's xstate module rather than @alan-nf's original issue. So far, I have only been able to reproduce the bug when using atomWithMachine.

I hope this is helpful in some way.

@Thisen
Copy link
Collaborator

Thisen commented Jun 27, 2022

Here's a reproduction on stackblitz of my state reset issue.

The bug I'm seeing might be related to jotai's xstate module rather than @alan-nf's original issue. So far, I have only been able to reproduce the bug when using atomWithMachine.

I hope this is helpful in some way.

I'm not sure how much I can do without a reproduction using only the core modules. @dai-shi, any ideas if we could have a problem with xstate module?

@dai-shi
Copy link
Member

dai-shi commented Jun 27, 2022

Here's a reproduction on stackblitz of my state reset issue.
The bug I'm seeing might be related to jotai's xstate module rather than @alan-nf's original issue. So far, I have only been able to reproduce the bug when using atomWithMachine.
I hope this is helpful in some way.

I'm not sure how much I can do without a reproduction using only the core modules.

I agree. We need someone help to reproduce it more simply.

any ideas if we could have a problem with xstate module?

atomWithMachine is a bit complicated, so it may cause the issue.
However, if so, I'm pretty sure we can reproduce it without atomWithMachine.
The use of .onMount might be related.

@alan-nf
Copy link
Author

alan-nf commented Jun 27, 2022

Part of the problem is that we've all been discussing somewhat separate issues:

  • In my real (Vite) project, I'm having two separate problems:
    • when files with atoms get HMR updated, my state gets reset. I don't know how to integrate jotai/babel/plugin-react-refresh with Vite so perhaps this is expected.
    • Vite is always HMR updating my files with state and I don't know why as they aren't getting edited
  • @bstro is reproducing a react refresh bug with atomWithMachine

So put another way, the problems are:

  1. What goes into a Vite hmr update is hard to debug
  2. It's not clear how to integrate Vite + Jotai in a way that enables react-refresh in files that have atoms in them
  3. There might be other bugs, e.g. with atomWithMachine

If you want a simple repro, here is one. This repro:

  1. Uses Vite + Jotai (only atom / useAtom)
  2. Has both the atom and UI in App.tsx. Updating App.tsx resets the counter.
  3. jotai/babel/plugin-react-refresh is not configured. I don't know how to configure it with Vite

This is the same code but the atom is moved to a separate file which lessens the problem with refresh (editing App.tsx does NOT reset the state, but editing state.ts still does reset the state)

@Thisen
Copy link
Collaborator

Thisen commented Jun 27, 2022

Part of the problem is that we've all been discussing somewhat separate issues:

  • In my real (Vite) project, I'm having two separate problems:

    • when files with atoms get HMR updated, my state gets reset. I don't know how to integrate jotai/babel/plugin-react-refresh with Vite so perhaps this is expected.
    • Vite is always HMR updating my files with state and I don't know why as they aren't getting edited
  • @bstro is reproducing a react refresh bug with atomWithMachine

So put another way, the problems are:

  1. What goes into a Vite hmr update is hard to debug
  2. It's not clear how to integrate Vite + Jotai in a way that enables react-refresh in files that have atoms in them
  3. There might be other bugs, e.g. with atomWithMachine

If you want a simple repro, here is one. This repro:

  1. Uses Vite + Jotai (only atom / useAtom)
  2. Has both the atom and UI in App.tsx. Updating App.tsx resets the counter.
  3. jotai/babel/plugin-react-refresh is not configured. I don't know how to configure it with Vite

This is the same code but the atom is moved to a separate file which lessens the problem with refresh (editing App.tsx does NOT reset the state, but editing state.ts still does reset the state)

There's docs in Vite on how to configure babel plugins:
https://github.com/vitejs/vite/tree/main/packages/plugin-react#babel-configuration

Have you tried that?

@alan-nf
Copy link
Author

alan-nf commented Jun 27, 2022

If I try to add the babel plugin on stackblitz:

export default defineConfig({
  plugins: [
    react({
      babel: {
        plugins: ['jotai/babel/plugin-react-refresh'],
      },
    }),
  ],
});

Error:

_babel_template.default is not a function

Error Details
[plugin:vite:react-babel] /home/projects/vitejs-vite-7ubsyk/src/main.tsx: _babel_template.default is not a function
/home/projects/vitejs-vite-7ubsyk/src/main.tsx
    at PluginPass.exit (file:///home/projects/vitejs-vite-7ubsyk/node_modules/jotai/esm/babel/plugin-react-refresh.mjs:56:57)
    at newFn (/home/projects/vitejs-vite-7ubsyk/node_modules/@babel/traverse/lib/visitors.js:177:21)
    at NodePath._call (/home/projects/vitejs-vite-7ubsyk/node_modules/@babel/traverse/lib/path/context.js:53:20)
    at NodePath.call (/home/projects/vitejs-vite-7ubsyk/node_modules/@babel/traverse/lib/path/context.js:40:17)
    at NodePath.visit (/home/projects/vitejs-vite-7ubsyk/node_modules/@babel/traverse/lib/path/context.js:109:8)
    at TraversalContext.visitQueue (/home/projects/vitejs-vite-7ubsyk/node_modules/@babel/traverse/lib/context.js:103:16)
    at TraversalContext.visitSingle (/home/projects/vitejs-vite-7ubsyk/node_modules/@babel/traverse/lib/context.js:77:19)
    at TraversalContext.visit (/home/projects/vitejs-vite-7ubsyk/node_modules/@babel/traverse/lib/context.js:131:19)
    at traverseNode (/home/projects/vitejs-vite-7ubsyk/node_modules/@babel/traverse/lib/traverse-node.js:24:17)
    at traverse (/home/projects/vitejs-vite-7ubsyk/node_modules/@babel/traverse/lib/index.js:62:34

If I try to do the same thing locally (repo on github), I get the error:

templateBuilder is not a function

Error Details
[vite:react-babel] /Users/anorbauer/src/jotai-vite-react-refresh/src/main.tsx: templateBuilder is not a function
file: /Users/anorbauer/src/jotai-vite-react-refresh/src/main.tsx
error during build:
TypeError: /Users/anorbauer/src/jotai-vite-react-refresh/src/main.tsx: templateBuilder is not a function
    at PluginPass.exit (file:///Users/anorbauer/src/jotai-vite-react-refresh/node_modules/jotai/esm/babel/plugin-react-refresh.mjs:41:34)
    at newFn (/Users/anorbauer/src/jotai-vite-react-refresh/node_modules/@babel/traverse/lib/visitors.js:177:21)
    at NodePath._call (/Users/anorbauer/src/jotai-vite-react-refresh/node_modules/@babel/traverse/lib/path/context.js:53:20)
    at NodePath.call (/Users/anorbauer/src/jotai-vite-react-refresh/node_modules/@babel/traverse/lib/path/context.js:40:17)
    at NodePath.visit (/Users/anorbauer/src/jotai-vite-react-refresh/node_modules/@babel/traverse/lib/path/context.js:109:8)
    at TraversalContext.visitQueue (/Users/anorbauer/src/jotai-vite-react-refresh/node_modules/@babel/traverse/lib/context.js:103:16)
    at TraversalContext.visitSingle (/Users/anorbauer/src/jotai-vite-react-refresh/node_modules/@babel/traverse/lib/context.js:77:19)
    at TraversalContext.visit (/Users/anorbauer/src/jotai-vite-react-refresh/node_modules/@babel/traverse/lib/context.js:131:19)
    at traverseNode (/Users/anorbauer/src/jotai-vite-react-refresh/node_modules/@babel/traverse/lib/traverse-node.js:24:17)
    at traverse (/Users/anorbauer/src/jotai-vite-react-refresh/node_modules/@babel/traverse/lib/index.js:62:34)

@Thisen
Copy link
Collaborator

Thisen commented Jun 27, 2022

If I try to add the babel plugin on stackblitz:

export default defineConfig({
  plugins: [
    react({
      babel: {
        plugins: ['jotai/babel/plugin-react-refresh'],
      },
    }),
  ],
});

Error:

_babel_template.default is not a function

Error Details
If I try to do the same thing locally (repo on github), I get the error:

templateBuilder is not a function

Error Details

Duplicate of #1251.

@alan-nf
Copy link
Author

alan-nf commented Jun 28, 2022

Duplicate of #1251.

👍 I subscribed to that thread. When a fix is released I will see if there are any other blockers to getting Vite + Jotai + React Refresh playing nicely together and will report back here. Thanks!

@Thisen
Copy link
Collaborator

Thisen commented Jun 30, 2022

@alan-nf This is solved in 1.7.3 :)

@alan-nf
Copy link
Author

alan-nf commented Jun 30, 2022

@Thisen I saw. I proofed it in the Stackblitz sandbox and it works perfectly there. I froze the sandbox and renamed it if you want to use it as an example/demo of vite + jotai working with react refresh.

I have a note to myself to verify that this fixes things in my actual work project. I will report back as soon as I verify that.

Thank you so much for fixing this. Should be a huge productivity booster.

@Thisen
Copy link
Collaborator

Thisen commented Jun 30, 2022

Glad to hear that!

@Thisen Thisen closed this as completed Jun 30, 2022
@alan-nf
Copy link
Author

alan-nf commented Jun 30, 2022

I'm seeing one more hiccup but I think it's to be expected. After a hot update, the atom update methods (result of useUpdateAtom) is a new function which triggers some of my useEffects in a cascading way. That's definitely to be expected and I just need to work around that, right?

Otherwise, this works great!

@dai-shi
Copy link
Member

dai-shi commented Jun 30, 2022

the atom update methods (result of useUpdateAtom) is a new function

Hm, not exactly sure, but if the atom reference is the same, the function should be the same. Can you create a minimal reproduction with codesandbox?

@alan-nf
Copy link
Author

alan-nf commented Jul 1, 2022

but if the atom reference is the same, the function should be the same. Can you create a minimal reproduction with codesandbox?

Is stackblitz okay? if so, here you go: https://stackblitz.com/edit/jotai-vite-react-refresh-fretjv?file=src%2FApp.tsx

Note that the atom reference is also changing as part of the hmr.

I don't think this is a bug. In the stackblitz repro I demonstrate that it happens with setState as well (both the state and updater are new references). If you act on such changes you just have to be aware that the references might update in an hmr. I think that assuming otherwise is probably a sign of bad code anyway?

@dai-shi
Copy link
Member

dai-shi commented Jul 2, 2022

Thanks.

https://stackblitz.com/edit/jotai-vite-react-refresh-fretjv?file=src%2FApp.tsx

"The updateInlineCount function was updated 12 times"
"The inlineCountAtom atom was updated 12 times"
"The useState updater was updated 12 times"

Yeah, looks like working perfectly.

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

4 participants