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

about use rx.memo with children #3279

Open
seewindcn opened this issue May 13, 2024 · 3 comments
Open

about use rx.memo with children #3279

seewindcn opened this issue May 13, 2024 · 3 comments

Comments

@seewindcn
Copy link
Contributor

Describe the bug
I am trying use memo for website's layout, like this:

@rx.memo
def layout1(children: rx.Component) -> rx.Component:
  from antd_demo.components import footer, navbar, header, subnav, content
  return layout.layout(
        header(),
        layout.layout(
            navbar(),
            layout.layout(
                subnav(),
                content(
                    children,
                )
            )
       )

it work; but the js like below:

<Layout1
            children={[xxxxxxxxxxxxxxxxxxx............]}>
            <RadixThemesFlex css={{"display": "flex"}}>
                <RadixThemesFlex align={`start`} css={{"height": "81vh", "width": "100%"}} >
                    <Reactmarkdown_6c4d2df3472374f2de5b35860d0c49c3/>
                    <RadixThemesFlex css={{"flex": 2, "justifySelf": "stretch", "alignSelf": "stretch"}}/>
                </RadixThemesFlex>
            </RadixThemesFlex>
        </Layout1>

my question is:

  1. why layout1 has children property? it is very long and useless. children is a special property in jsx.
  2. not doc for memo's usage;
  3. style of memo component isn't support;

Specifics (please complete the following information):

  • Python Version: 3.11
  • Reflex Version: 0.4.9
  • OS: ubuntu22.04
  • Browser (Optional):

Additional context
Add any other context about the problem here.

@picklelo
Copy link
Contributor

picklelo commented May 16, 2024

@seewindcn The memo component was a bit hacky and needs to be documented better for sure.

For your use case, the following code works for me:

import reflex as rx

@rx.memo
def layout1(children: rx.Component) -> rx.Component:
  return rx.container(
        rx.heading("heading"),
        children,
    )

def index():
    """The main view."""
    return layout1(
        rx.box("hi")
    )


app = rx.App()
app.add_page(index)

How are you passing in the children? Since it's a special prop, you should just pass it as a child like in normal reflex components, you don't have to specify the keyword argument.

For the styling - if you can create an issue for that, I think it's something we should be able to add.

@seewindcn
Copy link
Contributor Author

seewindcn commented May 16, 2024

yes, the code works; my question is about the index.js which generate from python:

 export function Layout1_cf15c797d04480712961c3b3fa2f9a5e () {
const state__state = useContext(StateContexts.state__state)                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                             
return (                                                                                                                                                                                                    
<Layout1 children={["<RadixThemesFlex .................................here is ..useless...........................................>
   here is ok.
</Layout1>

"<Layout1 children=" this children prop is useless, and maybe very very long. it may js file's size bigger.

@picklelo
Copy link
Contributor

@seewindcn Got it - I think this is just a bug in our memo, we should be recognizing children is not a normal prop, and it should put it within the children. We can keep this ticket open to track this, but seems like it's not a hard blocker at the moment.

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

No branches or pull requests

2 participants