-
Notifications
You must be signed in to change notification settings - Fork 502
Remove setSwapComponent
method and cleanup after PR #2379
#2383
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
Conversation
@pascalbaljet it was because react was complaining about using setCurrent outside of useEffect caused by the swapComponent called at first render, so checking the variable inside fixed the problema and the setSwapComponent can be safely removed |
@chack1172 Thanks for clarifying! |
Though the tests pass, navigation is broken in the Playground with this change, so let's see if there's a solution by using
|
I seem to have found a solution by moving the |
Closing this PR as it leads to other problems. Gonna have a fresh look at it. |
Moved the variables back outside of the App component again, as well as the |
It seems working fine! |
This is a cleanup of PR #2379.
isRouterInitialized
has been renamed torouterIsInitialized
.TherouterIsInitialized
bool was enough to fix the problem. The additionalsetSwapComponent
logic doesn't seem to provide any benefits. Perhaps this was needed beforecurrentIsInitialPage
was introduced, but I can't reproduce it.setSwapComponent
has been removed to prevent introducing another public method on the core router. The swapping of theswapComponent
method is now handled internally inApp.ts
.swapComponent
is now simply a variable that gets replaced in theuseEffect
hook.manual-visits.spec.ts
) but in the React implementation it was using asetTimeout
to overcome the initial problem. ThesetTimeout
is removed now.