-
-
Notifications
You must be signed in to change notification settings - Fork 635
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
HackerNews demo: "Back" button from story to main page is broken #25
Comments
What platform & browser is it broken on? Seems to be working expectedly on my side in Windows + Latest Brave (Chromium). Edit: It seems like the example breaks down after a few back-and-forward between the list and the news comments page, so that may be the real issue here. |
Yeah I think your edited version is correct: if you toggle back and forth between the list and news comments a few times, it breaks. Here's my readout of the Chrome error message.
|
Yeah debugging is made worse for me by the fact that all panics seem to give me a "RuntimeError: unreachable" instead of the actual panic message. The problem itself seems to come from
Is it possible that you would have nested update calls here? It seems odd otherwise that it would be borrowed many times, but I don't have full context on how the framework is structured at this point so it's hard to tell. Oddly enough, changing this to |
Oh thanks this is really helpful. There are only two places where When disposing of a scope When creating a signal I think it must be the first one. The page navigation of the router is trying to clean up the scope when you leave the page and it's overlapping with a signal update somewhere. I'm assuming this is a logic issue in the router code somewhere; I don't think it can be fixed at the reactive library level at the moment. |
Okay I'm going to leave this for a little bit — I am working on an alternate API for route data loaders anyway here so I will wrap that up and see if it resolves the bug. |
This has been fixed by e903e84. It turns out everything I thought about this was basically wrong, but it led me to fixing at least one bug in the router and significantly increasing the robustness of the reactive system when things go wrong in a way that doesn't really mean we need to panic. Basically, the issue was that certain |
That makes sense. This is similar to react's setting state after component is unmounted and it also does nothing, except that it warns about it since it means you have a kind of "leak" in a component. Do you think an warning that it happened would be beneficial at all here? Is that even something that you can do anything about? E.g. by cancelling the async request. |
Yeah it makes sense to throw in some Canceling the in-flight request is an interesting one. The error in this case was from the request resolving; we do have an So what's probably best is to have something like |
No description provided.
The text was updated successfully, but these errors were encountered: