resend / react-email

💌 Build and send emails using React
https://react.email
MIT License
12.67k stars 583 forks source link

Tailwind classes do not work as prop of component #1250

Open arsistyle opened 4 months ago

arsistyle commented 4 months ago

Describe the Bug

I am creating some small components to which other tailwind classes can be added as porps, but they are not being visible, but if I passed any other word as a class it works.

Here is a little demo:

with Tailwind classes

import React from 'react'
import { Text, Container } from '@react-email/components'

import { Main } from '../src/layout/CleanLayout'

const ComponentX = ({className}) => {
  return <p className={`mb-8 text-red-500 ${className}`}>Im a component</p>
}

export default function Email() {
  return (
    <Main>
      <Container className="px-4">
        <Text className="text-3xl">Hello world</Text>
        <ComponentX className="text-blue-500 bg-black" />
      </Container>
    </Main>
  )
}

and this is the output of the component:

<p class="undefined" style="margin-bottom:32px;color:rgb(239,68,68)">Im a component</p>

with Tailwind classes and random words

import React from 'react'
import { Text, Container } from '@react-email/components'

import { Main } from '../src/layout/CleanLayout'

const ComponentX = ({className}) => {
  return <p className={`mb-8 text-red-500 ${className}`}>Im a component</p>
}

export default function Email() {
  return (
    <Main>
      <Container className="px-4">
        <Text className="text-3xl">Hello world</Text>
        <ComponentX className="text-blue-500 bg-black foo bar" />
      </Container>
    </Main>
  )
}

and this is the output of the component:

<p class="foo bar" style="margin-bottom:32px;color:rgb(239,68,68)">Im a component</p>

Which package is affected (leave empty if unsure)

@react-email/tailwind

Link to the code that reproduces this issue

i dont have

To Reproduce

Just create a simple component like example on description

Expected Behavior

I expect that tailwind classes can be passed as properties of a component.

What's your node version? (if relevant)

20.11.0

gabrielmfern commented 4 months ago

I think this might be happening because they are being turned into styles to your component and passing them as props. Try taking a style prop into the underlying element for ComponentX to see what happens.

I also saw something related to this when someone was asking about how they could use tailwind-merge with our Tailwnd component. This is certainly a bug that causes unexpected behavior, some case we are missing to add treatment for, but these kinds of experiments might reveal a bit of insight into what is happening here.

arsistyle commented 4 months ago

this way?

because i have the same behavior

const ComponentX = ({ className }) => {
  return (
    <p className={`mb-8 text-red-500`}>
      <span className={className}>Im a component</span>
    </p>
  )
}

export default function Email() {
  return (
    <Main>
      <Container className="px-4">
        <Text className="text-3xl">Hello world</Text>
        <ComponentX className="text-blue-500 bg-black foo bar" />
      </Container>
    </Main>
  )
}
<p style="margin-bottom:32px;color:rgb(239,68,68)">
  <span class="foo bar">Im a component</span>
</p>
gabrielmfern commented 4 months ago

I mean like this

const ComponentX = ({ className, style }) => {
  return (
    <p className={`mb-8 text-red-500 ${className}`} style={style}>
      Im a component
    </p>
  )
}

export default function Email() {
  return (
    <Main>
      <Container className="px-4">
        <Text className="text-3xl">Hello world</Text>
        <ComponentX className="text-blue-500 bg-black foo bar" />
      </Container>
    </Main>
  )
}
arsistyle commented 4 months ago

Well, it works only if the added class does not exist before, see class text.

const ComponentX = ({ className, style }: { className?: string; style?: any }) => {
  return (
    <p className={`mb-8 text-red-500 ${className}`} style={style}>
      Im a component
    </p>
  )
}

export default function Email() {
  return (
    <Main>
      <Container className="px-4">
        <Text className="text-3xl">Hello world</Text>
        <ComponentX className="text-blue-500 bg-black foo bar" />
      </Container>
    </Main>
  )
}
<p class="foo bar" style="color:rgb(239,68,68);background-color:rgb(0,0,0);margin-bottom:32px">Im a component</p>

the class text-blue-500 does not apply because text-red-500 exists.

image

gabrielmfern commented 4 months ago

Ok, so this seems like another issue, then two things break this:

  1. Components without a style prop that have classNames do not work properly
  2. Styles from class names do not take priority based on the same order Tailwind does

I think your second issue there also would be helped a lot by being able to use tailwind merge here.

arsistyle commented 4 months ago

Well yes, tailwind-merge solved my second problem, thanks for that suggestion.

About the first problem, it's a bit tedious, but it's not something that keeps me awake at night, so I'll wait quietly if they can fix this bug.

Thanks for the help!

gabrielmfern commented 4 months ago

What we currently do on the Tailwind component is do a pass over the JSX tree while inlining styles for everything, including components.

This is probably what is generally harmful here, maybe what we should do instead is inline styles for only elements and then, for components define a wrapper component around them that uses Tailwind on its resolved children only.

Lmk if this makes sense in your situation here, tailwind merge's behavior is also something to consider but I also think this should get that up and running as expected as well.

gabrielmfern commented 4 months ago

Waiting on #1124 to be merged before working on this.

kotAPI commented 1 month ago

I have the same issue, tailwind color classes dont apply when above pattern is followed. Its becoming very difficult to build components that can be reused with this bug around.

kotAPI commented 1 month ago

Weirdly, for me this somehow worked - for my Text wrapper component

const Text = ({ underline, className = "", italic, bold, children, ...props }) => {
    var finalClasses = twMerge(className, underline ? 'underline' : '', italic ? 'italic' : '', bold ? 'font-bold' : '', commonStyleClasses, "inline")

    return <Text className={finalClasses}
        {...props}
    >{children}</Text>
}

I have no idea how or why this works, would be great if you can shed some light, as its very counter intuitive and is very different from how developers usually build components

WillsB3 commented 1 month ago

I'm also seeing this.

If i make a component like this:

export const Paragraph = ({
  children,
  className,
  style,
  strong,
  ...props
}: ParagraphProps) => {
  const classes = cn(
    'text-black text-m font-normal not-italic leading-normal',
    { 'font-bold': strong },
    className
  )

  if (props.test) {
    console.log('[P] classes', classes)
  }

  return (
    <Text className={classes} style={style} {...props}>
      {children}
    </Text>
  )
}

and then use it like this:

<P strong test="true" className="font-semibold">
  If you need to reschedule or cancel your call
</P>

The rendered paragraph has font-weight 700 instead of 600 and I see this in the terminal:

[P] classes text-m not-italic leading-normal font-semibold
[P] classes text-m not-italic leading-normal font-bold

So it looks like the first render(?) has the correct class, but the second does not…

kotAPI commented 1 month ago

I'm also seeing this.

If i make a component like this:

export const Paragraph = ({
  children,
  className,
  style,
  strong,
  ...props
}: ParagraphProps) => {
  const classes = cn(
    'text-black text-m font-normal not-italic leading-normal',
    { 'font-bold': strong },
    className
  )

  if (props.test) {
    console.log('[P] classes', classes)
  }

  return (
    <Text className={classes} style={style} {...props}>
      {children}
    </Text>
  )
}

and then use it like this:

<P strong test="true" className="font-semibold">
  If you need to reschedule or cancel your call
</P>

The rendered paragraph has font-weight 700 instead of 600 and I see this in the terminal:

[P] classes text-m not-italic leading-normal font-semibold
[P] classes text-m not-italic leading-normal font-bold

So it looks like the first render(?) has the correct class, but the second does not…

yup, same behaviour - kind of goes against the purpose of building your own components, hope this gets fixed sometime soon

WillsB3 commented 3 weeks ago

@gabrielmfern Am I right in thinking that we need https://github.com/resend/react-email/pull/1383 merged before this issue can be fixed? Do you think any extra changes will be required on top of those included in #1383 to fix the problem outlined in this issue?

gabrielmfern commented 3 weeks ago

@WillsB3 I actually have a fix for this on a branch, but there's no PR for it yet. It requires #1383 to be merged for it to work properly.