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

fix: HMR Break when using context in react templates - Issues: #3301 and #2624 #8329

Closed

Conversation

adnanalbeda
Copy link

@adnanalbeda adnanalbeda commented May 25, 2022

When using createContext and wrap elements with context.provider, hmr breaks and the context tries to access disposed object.

This new template avoid the problem by creating one react root per model, and it makes loading page faster too.

Description

HMR breaks in react templates when using contexts and wrap the <App /> with any context provider.

I found this error in console:

You are calling ReactDOMClient.createRoot() on a container that has already been passed to createRoot() before.

so it seems that in the current template, the root is being recreated with each Hot Reload leading to the dispose of old one, which made contexts stores throw can't access lexical declaration 'SomeContext' before initialization.

So the new main.jsx and main.tsx address this problem in general.
It makes sure that the root is created only once.
I also noticed pages are loading faster after using this method.

BTW, the react error appears in the console anyway after each Hot Reload, whether using context or not.

Additional context

This PR for the new template is a work around for issues #3301 and #2624.
It doesn't doesn't address the problem itself in the react-vite plugin.

Even though the problem doesn't persist, this solution creates a new issue.
The context resets with every new change, which means it loses all its data;
but still better than just seeing an empty white screen.

So until the issue is fixed in the plugin, this workaround does the job.


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

When using `createContext` and wrap elements with `context.provided`, hmr breaks and the context tries to access disposed object.

This code avoid the problem by creating one react root per model, and it makes loading page faster too.
@patak-dev
Copy link
Member

Thanks for the PR. IMO we shouldn't add a workaround in the templates. We should fix the HMR issue.

@adnanalbeda
Copy link
Author

adnanalbeda commented Jun 1, 2022

I don't know how I can exactly investigate the issue.

But after I did more work on the project,
I found more information which might be interesting.

HMR reloads the whole document after applying this solution and
only if context is provided and the change is made in:

.tsx or .ts files calling/using the context.
.tsx or .ts files which declare/export the context.
.tsx or .ts files imported in context modules.

I know this happens because Vite handles each file alone on calling it, and not as a bundled package.

So if I make an edit in ts or tsx file which has nothing to do with any context, everything will be fine, and the changes happen without a complete reload of the document, only the updated file will be rerendered.

Even if its data are coming from the context, if they are passed as props, HMR won't refresh the whole document.

@liho00
Copy link

liho00 commented Jul 1, 2022

@patak-dev can we merge this as temporary fix, since i research this issue has been persist long time ago, and it affecting the development experience, as we had to manually refresh everytime when we hot reload...

@sapphi-red
Copy link
Member

Closing as this is fixed by #10239.

@sapphi-red sapphi-red closed this Nov 4, 2022
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

Successfully merging this pull request may close these issues.

None yet

4 participants