Handle rx.State subclasses defined in function #4129
Merged
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
* create a new container module: `reflex.istate.dynamic` to save references to dynamically generated substates. * for substates with `<locals>` in the name, copy these to the container module and update the name to avoid duplication. * add test for "poor man" ComponentState Fix #4128
Also use the original name when checking for the "mangled name" pattern when doing undeclared Var assignment checking.
| if proposed_name not in known_names: | ||
| break | ||
| proposed_name = f"{cls.__name__}_{ix}" | ||
| setattr(reflex.istate.dynamic, proposed_name, cls) |
There was a problem hiding this comment.
Doesn't this prevent garbage collection if we never clean up the refs from the container module?
There was a problem hiding this comment.
the refs are for dynamically generated classes, so while it's possible that we won't clean these class objects up, i think that's okay, given that they are probably only created where needed for the app. i don't see a use case where an app creates a bunch of dynamic classes and then doesn't use them (why did it create them in the first place?)
Kastier1
pushed a commit
that referenced
this pull request
Oct 23, 2024
* Handle rx.State subclasses defined in function * create a new container module: `reflex.istate.dynamic` to save references to dynamically generated substates. * for substates with `<locals>` in the name, copy these to the container module and update the name to avoid duplication. * add test for "poor man" ComponentState Fix #4128 * test_state: disable local def handling for dupe-detection test * Track the original module and name for type hint evaluation Also use the original name when checking for the "mangled name" pattern when doing undeclared Var assignment checking.
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.
reflex.istate.dynamicto save references to dynamically generated substates.<locals>in the name, copy these to the container module and update the name to avoid duplication.Fix #4128