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

`fontWeight` attribute assignable to `string` | `number` #113

Closed jlarmstrongiv closed 4 months ago

jlarmstrongiv commented 4 months ago

I’m using some duplicate properties in inline styles (e.g. React.CSSProperties) and mjml props (e.g. IMjmlTextProps).

However, they are unable to share fontWeight as React.CSSProperties expects a string, while IMjmlTextProps expects a number. It would be great to add | number in all interfaces where fontWeight is defined.

Related

jlarmstrongiv commented 4 months ago

A quick fix is:

In your node_modules/@faire/mjml-react folder, run:

# avoid mac and linux inconsistencies with perl https://stackoverflow.com/a/54932255
find . -name '*.ts' -exec perl -pi -e 's/fontWeight\?: string/fontWeight\?: string | number/g' {} \;

Followed by running in your root package directory:

npx patch-package @faire/mjml-react

Resulting in the patches/@faire+mjml-react+3.3.0.patch:

diff --git a/node_modules/@faire/mjml-react/mjml/MjmlAccordionText.d.ts b/node_modules/@faire/mjml-react/mjml/MjmlAccordionText.d.ts
index b37f38e..dfaa9e5 100644
--- a/node_modules/@faire/mjml-react/mjml/MjmlAccordionText.d.ts
+++ b/node_modules/@faire/mjml-react/mjml/MjmlAccordionText.d.ts
@@ -3,7 +3,7 @@ export interface IMjmlAccordionTextProps {
     backgroundColor?: React.CSSProperties["backgroundColor"];
     fontSize?: string | number;
     fontFamily?: string;
-    fontWeight?: string;
+    fontWeight?: string | number;
     letterSpacing?: string | number;
     lineHeight?: string | number;
     color?: React.CSSProperties["color"];
diff --git a/node_modules/@faire/mjml-react/mjml/MjmlButton.d.ts b/node_modules/@faire/mjml-react/mjml/MjmlButton.d.ts
index f0c42c2..7e98723 100644
--- a/node_modules/@faire/mjml-react/mjml/MjmlButton.d.ts
+++ b/node_modules/@faire/mjml-react/mjml/MjmlButton.d.ts
@@ -16,7 +16,7 @@ export interface IMjmlButtonProps {
     fontFamily?: string;
     fontSize?: string | number;
     fontStyle?: string;
-    fontWeight?: string;
+    fontWeight?: string | number;
     height?: string | number;
     href?: string;
     name?: string;
diff --git a/node_modules/@faire/mjml-react/mjml/MjmlNavbarLink.d.ts b/node_modules/@faire/mjml-react/mjml/MjmlNavbarLink.d.ts
index 300ed42..5f81b87 100644
--- a/node_modules/@faire/mjml-react/mjml/MjmlNavbarLink.d.ts
+++ b/node_modules/@faire/mjml-react/mjml/MjmlNavbarLink.d.ts
@@ -5,7 +5,7 @@ export interface IMjmlNavbarLinkProps {
     fontFamily?: string;
     fontSize?: string | number;
     fontStyle?: string;
-    fontWeight?: string;
+    fontWeight?: string | number;
     href?: string;
     name?: string;
     /** MJML default value: _blank */
diff --git a/node_modules/@faire/mjml-react/mjml/MjmlSocial.d.ts b/node_modules/@faire/mjml-react/mjml/MjmlSocial.d.ts
index 1a74d6e..dfd216c 100644
--- a/node_modules/@faire/mjml-react/mjml/MjmlSocial.d.ts
+++ b/node_modules/@faire/mjml-react/mjml/MjmlSocial.d.ts
@@ -9,7 +9,7 @@ export interface IMjmlSocialProps {
     fontFamily?: string;
     fontSize?: string | number;
     fontStyle?: string;
-    fontWeight?: string;
+    fontWeight?: string | number;
     iconSize?: string | number;
     iconHeight?: string | number;
     iconPadding?: string | number;
diff --git a/node_modules/@faire/mjml-react/mjml/MjmlSocialElement.d.ts b/node_modules/@faire/mjml-react/mjml/MjmlSocialElement.d.ts
index b194739..9517a4d 100644
--- a/node_modules/@faire/mjml-react/mjml/MjmlSocialElement.d.ts
+++ b/node_modules/@faire/mjml-react/mjml/MjmlSocialElement.d.ts
@@ -9,7 +9,7 @@ export interface IMjmlSocialElementProps {
     fontFamily?: string;
     fontSize?: string | number;
     fontStyle?: string;
-    fontWeight?: string;
+    fontWeight?: string | number;
     href?: string;
     iconSize?: string | number;
     iconHeight?: string | number;
diff --git a/node_modules/@faire/mjml-react/mjml/MjmlTable.d.ts b/node_modules/@faire/mjml-react/mjml/MjmlTable.d.ts
index 1527c19..b3aac21 100644
--- a/node_modules/@faire/mjml-react/mjml/MjmlTable.d.ts
+++ b/node_modules/@faire/mjml-react/mjml/MjmlTable.d.ts
@@ -13,7 +13,7 @@ export interface IMjmlTableProps {
     color?: React.CSSProperties["color"];
     fontFamily?: string;
     fontSize?: string | number;
-    fontWeight?: string;
+    fontWeight?: string | number;
     lineHeight?: string | number;
     paddingBottom?: string | number;
     paddingLeft?: string | number;
diff --git a/node_modules/@faire/mjml-react/mjml/MjmlText.d.ts b/node_modules/@faire/mjml-react/mjml/MjmlText.d.ts
index df671de..24c227d 100644
--- a/node_modules/@faire/mjml-react/mjml/MjmlText.d.ts
+++ b/node_modules/@faire/mjml-react/mjml/MjmlText.d.ts
@@ -9,7 +9,7 @@ export interface IMjmlTextProps {
     fontFamily?: string;
     fontSize?: string | number;
     fontStyle?: string;
-    fontWeight?: string;
+    fontWeight?: string | number;
     height?: string | number;
     letterSpacing?: string | number;
     lineHeight?: string | number;

But, it would be much better to have a permanent solution

emmclaughlin commented 4 months ago

Hi @jlarmstrongiv 🙂

@faire/mjml-react uses a script (generate-mjml-react) to generate these type files. In that script we import the mjml component from mjml and determine its types based on the code definition. As mentioned in https://github.com/Faire/mjml-react/pull/81, mjml defines font-weight as a string here. We could add an override in the mjml-react script but the main goal of mjml-react is to have the type safety be aligned with what mjml is expecting.

It looks like in the mjml docs mj-text says it takes a number for font-weight, so one of them is possibly a mistake. I think the best permanent solution to this would be to adjust the mjml code to be 'string | number' if mjml truly accepts both for the font-weight.