aanckar / react-pdf-tailwind

Use Tailwind CSS to style PDFs created with react-pdf
315 stars 11 forks source link

Warn when invalid strings are passed #11

Closed chrissantamaria closed 1 year ago

chrissantamaria commented 1 year ago

I find myself occasionally passing invalid strings to the main tw function, resulting in a functional noop of no styles being applied. My current debugging approach is generally to log the tw output and ensure I'm getting back the style object I'm expecting.

It might be nice if this library could somehow indicate to consumers (either via console.warn or a thrown error) that one/many of the classes provided are invalid. Normal web-based Tailwind contexts can't normally do this since non-Tailwind classes could be passed to class attributes, but given that input strings to tw should all be valid we could be a bit more strict.

Open to thoughts on how/when these sorts of warnings should be provided. If this feels out of scope of this library, no worries - happy to implement in my own fork :)

aanckar commented 1 year ago

I agree that this library should do these checks (as you noticed in a previous issue, I initially had a console.warn in there).

IMO, the ideal solution would be if the tw function was fully typesafe, but this is of course harder to implement than simple runtime checks. I think I'm going to experiment a bit around this - but in the meantime, I think throwing errors is OK, so if you decide to implement this, feel free to open a PR :)

chrissantamaria commented 1 year ago

A static typechecking approach would be great! Not sure how far we can get with TypeScript wizardry of string literals though.

Reminds me a bit of typewind, though their approach effectively uses typed object properties with a compile-time transform back to strings. Maybe the strictness of react-pdf styles could make it a bit more viable here?

re: runtime warnings, are you good with throwing errors in all cases or would an (optional) config option be preferred? Maybe something like:

const tw = createTw({
  theme: {
    // ...
  },
  throwOnInvalidStyle: true,
});

// or if we'd rather leave the first argument reserved as a Tailwind config
const tw = createTw(
  {
    theme: {
      // ...
    },
  },
  {
    throwOnInvalidStyle: true,
  }
);

tw('foo'); // will throw
aanckar commented 1 year ago

I just published v2.0.0 which logs invalid class names with console.warn. For my use cases this is enough, do you have a specific scenario where throwing would be required/preferred?

chrissantamaria commented 1 year ago

Nope, console.warn works for me! Figured it might be useful to keep as an option but I think this is good enough for now. Think we're good to close this - thanks!