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

Prevent Substate class shadowing #1827

Merged
merged 1 commit into from
Sep 19, 2023
Merged

Prevent Substate class shadowing #1827

merged 1 commit into from
Sep 19, 2023

Conversation

ElijahAhianyo
Copy link
Collaborator

@ElijahAhianyo ElijahAhianyo commented Sep 18, 2023

This PR aims to prevent shadowing children substate which inherently leads to unexpected behaviors since they're not exactly shadowed. With this PR the following example will throw an error:

import reflex as rx

class State(rx.State):
       ...

class ChildState(State):
       ...

class ChildState(State):
      ... 

@ElijahAhianyo ElijahAhianyo marked this pull request as ready for review September 19, 2023 14:34
Copy link
Contributor

@picklelo picklelo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice fix!

@picklelo picklelo merged commit 2750228 into main Sep 19, 2023
@PlutarcoII
Copy link

This PR aims to prevent shadowing children substate which inherently leads to unexpected behaviors since they're not exactly shadowed. With this PR the following example will throw an error:

import reflex as rx

class State(rx.State):
       ...

class ChildState(State):
       ...

class ChildState(State):
      ... 

In reflex docs substate is an option when your app is growing up.. https://reflex.dev/docs/state/substates/
Could you please provide an example how-to do this with this new update?

@masenf
Copy link
Collaborator

masenf commented Sep 22, 2023

@PlutarcoII the examples in the docs should still work. This PR just prevents substates from having the exact same name due to how the state tree is implemented internally. It's possible that in the future this restriction may be lifted, but for now, just use different class names for each substate and it will work.

@PlutarcoII
Copy link

@masenf I arrive here due to an error in my code when I upgraded to reflex 0.2.8, but I didn't realize that I had two classes with the same name in different files. Thanks for your clarification.

Alek99 pushed a commit that referenced this pull request Sep 26, 2023
@picklelo picklelo deleted the elijah/substate_shadow branch October 9, 2023 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants