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

Missing interface prop on `MjmlConditionalComment` #109

Closed gvonkoss closed 5 months ago

gvonkoss commented 7 months ago

Hey folks, I'm pointing out a little niggle that I've found with the interface declaration for the MjmlConditionalComment component, that complains aboutchildren not being allowed. I'll open a PR with a fix but figured I would point it at an issue!

Thanks for maintaining this :v:

emmclaughlin commented 5 months ago

Hi @gvonkoss, I took a deeper look into this and I am not sure this is a @faire/mjml-react issue. I can get a test to pass with children as a prop in both scenarios. E.g.

    it("should render", () => {
      const comment = (
        <MjmlConditionalComment>
          First, solve the problem. Then, write the code.
        </MjmlConditionalComment>
      );
      expect(renderToMjml(comment)).toBe(
        "<mj-raw><!--[if gte mso 9]>First, solve the problem. Then, write the code.<![endif]--></mj-raw>"
      );
    });
    it("should render with children as prop", () => {
      const comment = (
        // eslint-disable-next-line react/no-children-prop
        <MjmlConditionalComment children="First, solve the problem. Then, write the code." />
      );
      expect(renderToMjml(comment)).toBe(
        "<mj-raw><!--[if gte mso 9]>First, solve the problem. Then, write the code.<![endif]--></mj-raw>"
      );
    });

I suspect what is not liking the children prop is the linter (or something similar) preferring children to be nested between open/closing tags (see // eslint-disable-next-line react/no-children-prop)