Fix subscriptions for read only signals passed as props #3173
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I can confirm that this fixed my original problem. The secondary problem that I mentioned on the bug report, however, seems to have become worse, as now I get a panic -- that does not appear to be coming from my code: |
@rogusdev without any way to reproduce the issue, there isn't much I can do to debug the second issue. Can you create a reproduction? |
|
I was hoping the stack trace would pop a lightbulb for you. If not, I support merging this as it is, because it is simple enough and does solve one problem. And then I will find a repro for the other thing. Thanks |
|
I misread the backtrace. We can just ignore subscriptions to dropped reactive contexts. I think the panic should be fixed, but I'm still not sure what is causing the missing subscriptions |
|
Your latest does indeed fix the panic and now I can use this! I may have found a repro for a reactive context dropped error, which may or may not be the same / related error. I will create that as a separate issue tho, as the original concerns I had seem to be resolved with these changes and would highly encourage them to get merged. |
|
All that said, the repro app I put in the issue does still give this warning (twice, immediately) when following the steps (click on 1 then click on 2) -- tho at least it now displays properly! Maybe that might help you investigate this better? |
|
I am good for this getting merged but @ealmloff or @jkelleyrtp what should we do about that warning I'm getting now? The repro on the issue triggers it completely consistently. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes subscriptions being cleared after changing values that are converted to ReadOnlySignal at a component boundary. It also adds a fuzzing test for Rc generational box values
Fixes #3147