Faire / mjml-react

React component library to generate the HTML emails on the fly
https://www.npmjs.com/package/@faire/mjml-react
MIT License
375 stars 16 forks source link

mjml-react does not seem to work in NextJS #106

Closed bduff9 closed 9 months ago

bduff9 commented 11 months ago

Hello! I have been using mjml for quite some time and recently started a new project with nextJS 13 and this library. Unfortunately, something seems to be going wrong and I get an empty MJML string every time when I run renderToMjml:

<mjml owa="desktop"><mj-head></mj-head><mj-body></mj-body></mjml>

I was able to recreate a minimal example only using the latest version of next and the latest version of this library here: https://codesandbox.io/p/sandbox/nextjs-13-mjml-4yktfz

Any idea on why this is happening or how to fix? Thank you!

emmclaughlin commented 11 months ago

Hi @bduff9 , thank you for reporting the problem. I have not worked with nextJS 13 too much, so I am not immediately sure what is going on here. I did narrow it down a bit more though. I tried to defined:

  const mjmlContent = (
    <MjmlSection>
      <MjmlColumn>
        <MjmlText>Hello world</MjmlText>
      </MjmlColumn>
    </MjmlSection>
  );

  const htmlContent = (
    <div>
      <p>
        <a>Hello world</a>
      </p>
    </div>
  );

  const mjmlStaticMarkup = ReactDOMServer.renderToStaticMarkup(mjmlContent);
  const htmlStaticMarkup = ReactDOMServer.renderToStaticMarkup(htmlContent);

and got back

mjmlStaticMarkup -> <mj-section></mj-section>
htmlStaticMarkup -> <div><p><a>Hello world</a></p></div>

So something is going on with the MjmlReact components that is causing renderToStaticMarkup (what renderToMjml uses under the hood) to not work in your example. I suspect it could be the React version, but I will have to dig a bit deeper. Just wanted to provide a quick update in case this context is helpful to you.

emmclaughlin commented 11 months ago

I tracked this down to a change in renderToStaticMarkup that I think started in react-dom v18 (I confirmed the error doesn't exist in react-dom v17). The change made was to change the functionality of how custom elements are handled, and it seems that the children prop is getting skipped for custom elements at this step.

The quickest solution to your issue will be either:

As far as the long term solution, I am not certain yet. It seems like the naming of the mjml elements (e.g. mj-column, mj-head) clash with the naming structure of custom elements, as the check for custom element is just if there is a hyphen. I see our options as either:

bduff9 commented 11 months ago

@emmclaughlin Thanks for looking into this. Unfortunately, downgrading to React 17 is not an option. I believe Next 13 requires React 18 but even if it doesn't, we use several React 18 features such as Suspense.

As far as using a different method to convert JSX to a string, do you have any suggestions for that? It's not an area I have looked at much previously.

emmclaughlin commented 11 months ago

I have not looked into it much. From a quick search here are some potential options to try:

bduff9 commented 11 months ago

@emmclaughlin Thanks, I saw those too. Unfortunately, the first and third options only render the JSX string, not the HTML markup. The middle option just uses renderToStaticMarkup like mjml-react so that's a no-go too. I'll keep looking though, would love to use this library. Thanks again.

emmclaughlin commented 11 months ago

@bduff9 I was worried that would be the case. I will keep looking to see if there is any other quick workaround that can unblock you here and let you know if I find anything

Palmik commented 11 months ago

For now I have solved this by using pnpm add react-dom-17@npm:react-dom@^17.0.0. Likely you only ever render the emails server side, so there's no client-side bloat.

Also, if you are using app router like me (I have a simple previewer app for MJML react), then you want:

module.exports = {
  experimental: {
    serverComponentsExternalPackages: ['mjml']
  }
}

In your next.config.js, otherwise you get weird errors about uglify imports: https://github.com/vercel/next.js/issues/50042

emmclaughlin commented 10 months ago

Update from the React team: https://github.com/facebook/react/issues/27403#issuecomment-1769857291

Sounds like it should be fixed in the next release 🙌

emmclaughlin commented 9 months ago

Closing this issue as we have tested mjml-react with the latest React canary builds and their fix is sufficient to reconcile this issue.