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

Passing children as props to a react component breaks in production. #5493

Closed
1 task done
miklschmidt opened this issue Nov 29, 2022 · 10 comments · Fixed by #5886
Closed
1 task done

Passing children as props to a react component breaks in production. #5493

miklschmidt opened this issue Nov 29, 2022 · 10 comments · Fixed by #5886
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)

Comments

@miklschmidt
Copy link

miklschmidt commented Nov 29, 2022

What version of astro are you using?

1.6.11

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

yarn

What operating system are you using?

linux

Describe the Bug

If you're passing children as a prop to a react component from a .astro file, instead of doing it via markup, eg:

<MyComponent children="test" />

The component will receive {children: null} when building. It works fine in astro dev, but breaks in astro build when the island is loaded. Since this is perfectly viable React code, i'd regard this as a bug.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-l4gigd

Participation

  • I am willing to submit a pull request for this issue. As long as someone can point me in the right direction.
@miklschmidt miklschmidt changed the title Passing children as props to a react component does not work. Passing children as props to a react component breaks in production. Nov 29, 2022
@JerryWu1234
Copy link
Contributor

mark

@JerryWu1234
Copy link
Contributor

@miklschmidt
Copy link
Author

@wulinsheng123 I've read them. It doesn't mention this use case. If you weren't supposed to pass children as props, i'd expect an error.

@JerryWu1234
Copy link
Contributor

< MyComponent > Breaks in production </MyComponent >
try again which use this

@JerryWu1234
Copy link
Contributor

image

@miklschmidt
Copy link
Author

miklschmidt commented Dec 5, 2022

< MyComponent > Breaks in production
try again which use this

I'm perfectly aware that this works. I'm saying <MyComponent children="break in production"/> fails silently only in production. This is a bug.

@JerryWu1234
Copy link
Contributor

JerryWu1234 commented Dec 5, 2022

Ok,
I still want to confirm one thing with you: the error seems to happen whether the development or the production on my side
Do you only happen in the production?
image
this was your offers project

@miklschmidt
Copy link
Author

miklschmidt commented Dec 5, 2022

Yup, you get this because of the bug in astro (SSR HTML is inconsistent with client rendered HTML), react then switches to client rendering and rerenders the component. In production it doesn’t, so the component is blank (gets serialized to children=null).

@matthewp
Copy link
Contributor

matthewp commented Dec 6, 2022

What happens in regular client-side (non-Astro) React when you pass it a prop named children with a string value?

@matthewp matthewp added - P2: has workaround Bug, but has workaround (priority) - P3: minor bug An edge case that only affects very specific usage (priority) and removed - P2: has workaround Bug, but has workaround (priority) labels Dec 6, 2022
@miklschmidt
Copy link
Author

miklschmidt commented Dec 7, 2022

What happens in regular client-side (non-Astro) React when you pass it a prop named children with a string value?

props.children is then a string, as you'd expect.

import React from "https://esm.sh/react@18";
import ReactDOM from "https://esm.sh/react-dom@18";

const Test = (props) => {
  return (
    <div>{props.children}</div>
  )
}

const App = () => {
  return(
    <>
      <div className="box">
        <h1>Passing string children as prop</h1>
        <Test children="prop" />
      </div>
      <div className="box">
        <h1>Passing string children both as prop and children</h1>
        <Test children="prop">children</Test>
      </div>
    </>
  );
}

ReactDOM.render(<App />,
document.getElementById("root"))

Screenshot_17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants