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

feat: minified state names #3701

Conversation

benedikt-bartscher
Copy link
Contributor

@benedikt-bartscher benedikt-bartscher commented Jul 24, 2024

followup of #3214

Copy link
Collaborator

@masenf masenf left a comment

Choose a reason for hiding this comment

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

this is great!

# The env var to toggle minification of states.
ENV_MINIFY_STATES = "REFLEX_MINIFY_STATES"
# Whether to minify states.
MINIFY_STATES = os.environ.get(ENV_MINIFY_STATES, False)
Copy link
Collaborator

Choose a reason for hiding this comment

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

i'm thinking we should minify state names by default in prod mode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. Should we also move this to rx.Config.minify_states?

Copy link
Collaborator

@masenf masenf left a comment

Choose a reason for hiding this comment

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

unfortunately trying to run it on reflex-web is giving a cryptic error

  File "/Users/masenf/code/reflex-dev/reflex-web/pcweb/pcweb.py", line 8, in <module>
    from pcweb.pages import page404, routes
  File "/Users/masenf/code/reflex-dev/reflex-web/pcweb/pages/__init__.py", line 3, in <module>
    from .blog import blog_routes
  File "/Users/masenf/code/reflex-dev/reflex-web/pcweb/pages/blog/__init__.py", line 1, in <module>
    from .blog import *
  File "/Users/masenf/code/reflex-dev/reflex-web/pcweb/pages/blog/blog.py", line 2, in <module>
    from pcweb.templates.webpage import webpage
  File "/Users/masenf/code/reflex-dev/reflex-web/pcweb/templates/__init__.py", line 1, in <module>
    from .docpage import docpage
  File "/Users/masenf/code/reflex-dev/reflex-web/pcweb/templates/docpage/__init__.py", line 1, in <module>
    from .docpage import *
  File "/Users/masenf/code/reflex-dev/reflex-web/pcweb/templates/docpage/docpage.py", line 545, in <module>
    class RadixDocState(rx.State):
  File "pydantic/main.py", line 282, in pydantic.main.ModelMetaclass.__new__
  File "<frozen abc>", line 106, in __new__
  File "/Users/masenf/code/reflex-dev/reflex/reflex/state.py", line 563, in __init_subclass__
    raise StateValueError(
reflex.utils.exceptions.StateValueError: The substate class 'a' has been defined multiple times. Shadowing substate classes is not allowed.

Copy link
Collaborator

@masenf masenf left a comment

Choose a reason for hiding this comment

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

okay, i found a few other problems that need to be addressed, but nothing that suggests this is the wrong approach.

reflex/constants/compiler.py Outdated Show resolved Hide resolved
reflex/state.py Outdated Show resolved Hide resolved
@benedikt-bartscher benedikt-bartscher changed the title wip minified state names feat: minified state names Jul 25, 2024
@benedikt-bartscher benedikt-bartscher marked this pull request as ready for review July 25, 2024 22:29
@benedikt-bartscher benedikt-bartscher marked this pull request as draft July 27, 2024 22:23
@benedikt-bartscher benedikt-bartscher force-pushed the minify-state-names branch 2 times, most recently from 3c1946d to 337f4cb Compare August 4, 2024 19:41
@benedikt-bartscher
Copy link
Contributor Author

Superseded by #3728

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.

2 participants