mjml-react icon indicating copy to clipboard operation
mjml-react copied to clipboard

Typescript: More specific types for children

Open osdiab opened this issue 4 years ago • 7 comments

This is not necessarily the right place to put this since typings are managed at DefinitelyTyped, but figured it would get more relevant eyeballs here so posting:

One of the great things about MJML is that it validates your email HTML. But it would be even better if some of those validations could occur at compile time instead.

There's a lot of low hanging fruit like "children of mj-sections need to be mj-columns" or "an h1 tag can only be inside of an mj-text instance", which can be checked before even running your code. Would be amazing if those were typed explicitly - ~~although I am not sure how to type "an instance of a specific component" off the top of my head~~ yeah i don't think it's possible :P

Exploring other ways it can potentially be done without messing with the API of this library

osdiab avatar Apr 24 '20 13:04 osdiab

Made an issue to see if it can be done at the lint level but I'm not optimistic :P doing this might require doing a non-JSX structure to make it easier to type, so maybe a fork of this library

osdiab avatar Apr 24 '20 14:04 osdiab

Can def happen at runtime with proptypes, and Flow can type it - but TypeScript is unable at the moment (possibility for late june Typescript 4.0 (https://github.com/microsoft/TypeScript/issues/38510#issuecomment-627895715), but the PR for it has perf issues so potentially not) - discussions at typescript here:

  • https://github.com/microsoft/TypeScript/issues/13890
  • https://github.com/microsoft/TypeScript/issues/14729
  • https://github.com/microsoft/TypeScript/issues/21699

osdiab avatar May 18 '20 01:05 osdiab

Typescript allows checking children type https://www.typescriptlang.org/docs/handbook/jsx.html#children-type-checking . I guess this might be used.

daliusd avatar Sep 23 '20 14:09 daliusd

@daliusd that doesn't help unless the children are not actually react elements but just objects or primitives that mjml-react can use to build the react elements themselves, but that would be a dramatic and very breaking change to this library. for instance if you set the children prop to be an array of strings like {children: string[]} that works, but you can't have the children be specific instances of react components since the type of <AnyComponent /> will just be a JSX.Element type, which aren't distinguishable.

osdiab avatar Sep 24 '20 02:09 osdiab

for example to use that, instead of writing an MJML component like

<MjmlSection>
  <MjmlColumn />
</MjmlSection>

You would need to not use JSX for the children and do something like this:

<MjmlSection>
  {[
    {type: "MjmlColumn", contents: []}
  ]}
</MjmlSection>

and MjmlSection would need to know how to transform that children array into actual JSX elements. But that's very clumsy by React standards and I think would be best supported by just not using this library altogether lol

osdiab avatar Sep 24 '20 02:09 osdiab

If project owners are open to migration to TypeScript, I could help migrating all JavaScript into TypeScript in a PR.

  • just let me know, so I don't start something that you are not okay with :)

o-alexandrov avatar Oct 17 '20 06:10 o-alexandrov

Hey @o-alexandrov we are thinking about converting this project to a typescript. We are doing a little bit of research currently. Once finished we will let you know. Stay tuned.

mastertheblaster avatar Oct 20 '20 17:10 mastertheblaster

Hey, this project is no longer maintained so if someone wants to do something about it then feel free to fork it.

daliusd avatar Sep 05 '22 07:09 daliusd