wix-incubator / mjml-react

React component library to generate the HTML emails on the fly
MIT License
993 stars 50 forks source link

Urgent: Adjust component prop-types for use in MjmlAttributes #66

Closed IanEdington closed 2 years ago

IanEdington commented 3 years ago

Currently the prop-types assume most components have children. This is not the case in when used within the MjmlAttributes component. This PR removes the isRequired from all mjml components so they can be used inside the Atttributes.

This is an example email that is valid in MJML but not in mjml-react currently: https://mjml.io/try-it-live/k1kGrokirG2 (full email is at the bottom in case the link gets modified)

Disclaimer: I haven't used prop-types in ~4 years so I'm definitely not in the know of how to use them please correct me if I'm doing something wrong.

Why it's Urgent

This is urgent because I'm a maintainer on the DefinitelyTyped Types and the DefinitelyTyped maintainers wants us to conform to the types in this libraries prop-types. If we do it the way the prop-types are done currently we will throw a bunch of compile time errors for things that are allowed in MJML. ref: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/56254#discussion_r729291281

Example Email

<mjml>
  <mj-head>
    <mj-attributes>

      <mj-text>Head Components</mj-text>

      <mj-breakpoint width="50px" />
      <mj-font name="some-font" />
      <mj-attributes />
      <mj-html-attributes />
      <mj-preview />
      <mj-style inline="inline" />
      <mj-title />

      <mj-text>Body Components</mj-text>

      <mj-accordion mj-class="some-class" />
      <mj-button mj-class="some-class" />
      <mj-carousel mj-class="some-class" />
      <mj-column mj-class="some-class" />
      <mj-divider mj-class="some-class" />
      <mj-group mj-class="some-class" />
      <mj-hero mj-class="some-class" />
      <mj-image mj-class="some-class" />
      <mj-navbar mj-class="some-class" />
      <mj-raw mj-class="some-class" />
      <mj-section mj-class="some-class" />
      <mj-social mj-class="some-class" />
      <mj-spacer mj-class="some-class" />
      <mj-table mj-class="some-class" />
      <mj-text mj-class="some-class" />
      <mj-wrapper mj-class="some-class" />

      <mj-text>Weirder components that still work</mj-text>

      <mjml lang="en" />
      <mj-head />
      <mj-body mj-class="some-class" />

    </mj-attributes>
  </mj-head>
  <mj-body>
    <mj-section>
      <mj-wrapper>The red x to the left shows that validation is being done on this mjml</mj-wrapper>
      <mj-column>
        <mj-text mj-class="blue big">
          Hello World!
        </mj-text>
      </mj-column>
    </mj-section>
  </mj-body>
</mjml>
daliusd commented 2 years ago

Looks good to me.