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

Bug: renderToString and renderToStaticMarkup skipping children for HTML custom elements #27403

Closed
emmclaughlin opened this issue Sep 21, 2023 · 4 comments

Comments

@emmclaughlin
Copy link

React version: 18.2.0

Steps To Reproduce

  import ReactDOMServer from "react-dom/server";
 
  const customElements = (
    <custom-element-a>
      <custom-element-b></custom-element-b>
    </custom-element-a>
  );
  
  const staticMarkup = ReactDOMServer.renderToStaticMarkup(customElements);

  // staticMarkup resolves as <custom-element-a></custom-element-a>

The current behavior

In the current behavior, renderToString and renderToStaticMarkup skip prop types of functions and objects on custom elements (see here). However, the children prop is an object so that also gets skipped.

The expected behavior

  import ReactDOMServer from "react-dom/server";

  const customElements = (
    <custom-element-a>
      <custom-element-b></custom-element-b>
    </custom-element-a>
  );
  
  const staticMarkup = ReactDOMServer.renderToStaticMarkup(customElements);

  // staticMarkup expect to resolve as <custom-element-a><custom-element-b></custom-element-b></custom-element-a>

My proposed fix would be adding && propKey !== 'children' to this check so that children are not skipped. However, I am not sure if I am missing an alternative reason to not include children here.

@LucaDillenburg
Copy link

Hello @emmclaughlin !

I've tried to reproduce your issue, but couldn't yet. Can you provide a minimal reproduction?

For now, this is what I've tested:

import ReactDOMServer from "react-dom/server";

const CustomElementA = ({ children }) => (
    <div>
        <h1>Hello World</h1>
        {children}
    </div>
);

const CustomElementB = ({ children }) => <p>This is a text description.</p>;

const customElements = (
    <CustomElementA>
        <CustomElementB></CustomElementB>
    </CustomElementA>
);

export const staticMarkup = ReactDOMServer.renderToStaticMarkup(customElements);
console.log(staticMarkup);
console.log(ReactDOMServer.renderToString(staticMarkup));

The result is both custom elements being printed:

<div><h1>Hello World</h1><p>This is a text description.</p></div>

@emmclaughlin
Copy link
Author

emmclaughlin commented Sep 22, 2023

Hi @LucaDillenburg !

Thank you for the response. I should have definitely defined the problem better in my first post.

The custom element specifically is a html custom element . I'll update the title to reflect this as well.

The specific thing that causes the renderToString/renderToStaticMarkup to diverge is the hyphen (-). You can see the logic here.

I found the easiest way to reproduce would be to add a test case to ReactServerRendering-test.js . The test cause would be:

    it('children should be rendered for custom elements', () => {
      const output = ReactDOMServer.renderToString(
        <my-custom-element-a>
          <my-custom-element-b></my-custom-element-b>
        </my-custom-element-a>,
      );
      expect(output).toBe(`<my-custom-element-a><my-custom-element-b></my-custom-element-b></my-custom-element-a>`);
    });

The above will fail, as the output is actually <my-custom-element-a></my-custom-element-a> because children are skipped during the server rendering.

@emmclaughlin emmclaughlin changed the title Bug: renderToString and renderToStaticMarkup skipping children for custom elements Bug: renderToString and renderToStaticMarkup skipping children for HTML custom elements Sep 22, 2023
@sophiebits
Copy link
Collaborator

I believe this was just fixed in #27511! This fix will be in the next release and you can also use the latest canary build (I verified it outputs <custom-element-a><custom-element-b></custom-element-b></custom-element-a>.)

@emmclaughlin
Copy link
Author

Thank you all for the help here 🙏

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

3 participants