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

transformFlowNode does not skip null children, causing TypeError in hotReplacementRender #942

Closed
Jessidhia opened this issue Apr 24, 2018 · 8 comments
Assignees

Comments

@Jessidhia
Copy link

Jessidhia commented Apr 24, 2018

Description

What you are reporting: Hot reloading not working

Expected behavior

What you think should happen: Edited components should update

Actual behavior

What actually happens: React-hot-loader: reconcilation failed due to error TypeError: Cannot read property 'type' of null

Environment

React Hot Loader version: 4.1.1

Reproducible Demo

I've not been able to make one, but this is what the result of render(instance) looks like, before it goes through to transformFlowNode:

image

The actual crash happens later, on the if (_typeof(child.type) !== _typeof(stackChild.type)) check, because child (the 3rd item in the React.Fragment) is null.

The component that produces this tree that ultimately crashes is completely unrelated to the component that I updated to trigger the hot reload; other than sharing a distant parent component.


I believe the fix will be to also pass the node.props.children inside transformFlowNode through filterNullArray, but I'm not sure it is that easy. This avoids the crash, but I still get an unable to merge warning inside a React.createContext Provider, and the edited component's rendering is not updated unless I trigger its state to change through other means. 🤔

@theKashey theKashey self-assigned this Apr 24, 2018
@theKashey
Copy link
Collaborator

Real detective! Look like condition render in Fragment could break everything.

@theKashey
Copy link
Collaborator

Actually here we got two bugs.
SFC inside Context might not be "force" updated, as long there is no command to do it. The test I have actually testing Stateful component, the one I could update.

@gregberge
Copy link
Collaborator

@theKashey it looks like you fixed it? Isn't it?

@theKashey
Copy link
Collaborator

theKashey commented Apr 24, 2018

@neoziro - yep, now it is fixed.

@timneutkens
Copy link

This still happens when using Next.js: vercel/next.js#4299

The latest version of react-hot-loader does fix the error when using Fragments though.

@theKashey
Copy link
Collaborator

So just in time.

@timneutkens
Copy link

@theKashey seems like there’s still an issue where the component gets remounted after componentdidupdate and returning null in render. Do you have any idea how to fix that? I’m checking it out myself too, but you’re definitely more known with the codebase

@theKashey
Copy link
Collaborator

Could you elaborate more, or provide some example?
Theoretically, anything with "didUpdate", as something that will be triggered after RHL did its job, shall not affect anything.
Feel free just open a PR with a new test failing, and ask me to fix it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants