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

[Navigator] React Hot Loader and StaticContainer Usage in Navigator #1425

Closed
skevy opened this issue May 27, 2015 · 7 comments
Closed

[Navigator] React Hot Loader and StaticContainer Usage in Navigator #1425

skevy opened this issue May 27, 2015 · 7 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@skevy
Copy link
Contributor

skevy commented May 27, 2015

I'm opening this issue for discussion at the moment, because I'm not quite sure what the proper fix is yet.

React Native's Navigator uses a pattern that will return, if the component has already been rendered before, a StaticContainer element that has no children with the shouldUpdate prop set to false. This never breaks "in real life", because the whole point of not updating that component is to keep the previous reconciliation. The problem is, when using something like React Hot Loader, it force updates all components in the tree...so it will fail with an onlyChild invariant violation in StaticContainer when this pattern is used.

Can we think of any elegant way to keep Navigator working with React Hot Loader? Theoretically, in the check in NavigatorNavigationBar and NavigatorBreadcrumbNavigationBar that is determining whether to recalculate the children or not, if we could know whether or not the component is being forceUpdated, we could allow for the recalculation of the children. I'm just honestly not sure how to check for that, or if that's some kind of anti-pattern.

/cc @ericvicenti

@ide
Copy link
Contributor

ide commented May 27, 2015

@ericvicenti mentioned in another discussion that StaticX components can often be replaced by holding a reference to the JSX element, and returning the same element instance from render() each time it is called. That should be robust against forceUpdates too.

@skevy
Copy link
Contributor Author

skevy commented May 27, 2015

@ide do you have a link to that discussion?

@ide
Copy link
Contributor

ide commented May 27, 2015

#262 (comment)

@skevy
Copy link
Contributor Author

skevy commented May 27, 2015

Cool, so based on that, we should work on updating Navigator to use that new pattern, rather than the StaticContainer pattern

@skevy
Copy link
Contributor Author

skevy commented May 27, 2015

@ericvicenti I saw somewhere you were working on a refactor of Navigator...does it include this change?

@ericvicenti
Copy link
Contributor

Yep, we should totally migrate to the "freezing" technique of putting the rendered descriptor directly into the state and using it directly in later renders instead of StaticComponent

The change that will be coming in a few days is actually removing the contextual navigation capabilities from Navigator, because it is causing too much complication. A better and more isolated solution for contextual navigation will come later

@brentvatne brentvatne changed the title React Hot Loader and StaticContainer Usage in Navigator [Navigator] React Hot Loader and StaticContainer Usage in Navigator May 29, 2015
@skevy
Copy link
Contributor Author

skevy commented Nov 25, 2015

This doesn't apply anymore, as Navigator no longer uses StaticContainer. Closing.

@skevy skevy closed this as completed Nov 25, 2015
@facebook facebook locked as resolved and limited conversation to collaborators Jul 22, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

4 participants