feat(utils): allow object props to make it to the component

[robin-ray]

Hello!

Summary

This PR changes the behavior of convertPropValueToMjml to give users of this library more flexibility, at the expense of allowing some unintentional mistakes to happen (ex: accidentally rendering some-prop="[Object object]").

Motivation

I have built a small library that uses objects and toString() to build up expressions that can be rendered to an MJML template. This works great for props on ordinary React HTML elements (this is a contrived example):

const MyImage = () => {
  const imgURL = { toString: () => "/path/to/some/resource" };
  return <img src={imgURL} />;
};

React renders <img src="/path/to/some/resource" /> for the snippet above.

However, I have to explicitly convert the value to a string for it to pass through mjml-react:

const MyImage = () => {
  const imgURL = { toString: () => "/path/to/some/resource" };
  return <MjmlImage src={`${imgURL}`} />;
};

Without the explicit conversion, React will render <mj-image /> because convertPropValueToMjml does not allow object props through (except for dangerouslySetInnerHTML). It would be convenient if mjml-react components behaved more like React HTML components in this regard so that I don't have to do the extra string conversion.

Thanks for considering this PR!

[emmclaughlin]

Hi @robin-ray, and thank you for the idea.

Unfortunately I am not sure giving more flexibility at the cost of unintentional mistakes is something we want to do with regards to the type safety on the mjml-react components. However, it seems as though you are using javascript? I noticed that the src prop on the html img tag also throws a type error when trying to pass the imgURL object.

Essentially what I am thinking is it may be reasonable to relax checks in convertPropValueToMjml so that all props can pass through in javascript, but in typescript

const MyImage = () => {
  const imgURL = { toString: () => "/path/to/some/resource" };
  return <img src={imgURL} />;
};

will still throw an error.

[robin-ray]

That's right! I'm using JavaScript for my components to avoid that issue.

[robin-ray]

I'm all for having this be a compile-time error in TypeScript! My concern is about the runtime check.

[emmclaughlin]

In that case I completely agree, it is not a good development experience for props to magically disappear. Just running CI checks now

[IanEdington]

This makes sense to me.

In general we are a little to strict about the props coming into components IMO. This is a related ticket that I think we should do in v4: #43

This might be a braking change since anything that was being silently made null will now be passed through. Very unlikely though and it only breaks things that don't conform to the API so I think it's fine to add to v3. I've updated the title to a feat though.

[emmclaughlin]

Looks good to me, thank you for the PR

[github-actions]

🎉 This PR is included in version 3.2.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀