Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[starlight-tailwind]: CSS warnings when setting a custom white tone using the tailwind plugin #1716

Closed
1 task
Fryuni opened this issue Apr 6, 2024 · 5 comments · Fixed by #1726
Closed
1 task

Comments

@Fryuni
Copy link
Member

Fryuni commented Apr 6, 2024

What version of @astrojs/starlight-tailwind are you using?

2.0.1

What version of astro are you using?

4.5.16

What package manager are you using?

pnpm

What operating system are you using?

Linux

What browser are you using?

Chrome

Describe the Bug

When using the @astrojs/starlight-tailwind and configuring a custom white color, the build emits multiple warnings regarding CSS.

The config:

import starlightTailwind from '@astrojs/starlight-tailwind';
import colors from 'tailwindcss/colors';

/** @type {import('tailwindcss').Config} */
export default {
  content: ['./src/**/*.{astro,html,js,jsx,md,mdx,svelte,ts,tsx,vue}'],
  theme: {
    extend: {
      colors: {
        white: colors.stone,
      },
    },
  },
  plugins: [starlightTailwind()],
};

The warnings (truncated for brevity; it has about 20 of them):

21:52:50 [WARN] [vite] warnings when minifying css:
▲ [WARNING] Expected identifier but found "50" [css-syntax-error]

    <stdin>:14:4:
      14 │     50: #fafaf9;
         ╵     ~~


▲ [WARNING] Expected identifier but found "100" [css-syntax-error]

    <stdin>:15:4:
      15 │     100: #f5f5f4;
         ╵     ~~~


▲ [WARNING] Expected identifier but found "200" [css-syntax-error]

    <stdin>:16:4:
      16 │     200: #e7e5e4;

This problem only happens for the colors -> white field. I tested with colors -> accent and colors -> black; those fields don't have the same problem.

Passing other colors in that configuration also shows the same warning; it is not specific to Tailwind's Stone color.

The same configuration without the @astrojs/starlight-tailwind plugin shows no warnings.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-qea4d6?file=tailwind.config.mjs

Participation

  • I am willing to submit a pull request for this issue.
@delucis
Copy link
Member

delucis commented Apr 6, 2024

Ah, interesting. I think this is because we use theme('colors.white') to get #fff in the Starlight Tailwind plugin, e.g.

'--sl-color-white': theme('colors.white'),

The default Tailwind colors shape is something like this:

const colors = {
    transparent: "transparent",
    black: "#000",
    white: "#fff",
    gray: {
        50: "#f8fafc",
        100: "#f1f5f9",
        200: "#e2e8f0",
        // etc.
    },
    // other colors
}

So colors.white is expected to be a single string literal, not an object.

Usually when setting an alternate gray shade, this is done via gray: colors.stone. (This also makes sense logically because “white” and “black” are not categories that span a range of light to dark colours, but specific luminosities.)

Do you think we should support bending Tailwind’s palette naming or would a clear error for this case and not supporting it be acceptable?

@Fryuni
Copy link
Member Author

Fryuni commented Apr 6, 2024

But it works for black changing the pure black to a shade of the given color without any warnings, I think white should do the same, probably by taking the lightest shade of the color

@delucis
Copy link
Member

delucis commented Apr 6, 2024

But it works for black changing the pure black to a shade of the given color without any warnings, I think white should do the same, probably by taking the lightest shade of the color

Not really! Starlight’s colour scheme doesn’t use Tailwind’s black anywhere, it uses Gray 900 for its darkest shade. So if you set a different gray palette, it will use the 900 value from that palette. If you set a custom value for black in your Tailwind config, Starlight completely ignores it. But the white in Starlight is always 100% luminosity, i.e. #fff, so there is no variation between palettes.

If someone really, really wanted to have a different shade for their whites, they could set e.g. white: colors.stone.50. On the other hand, white: colors.stone is kind of a semantic “mistake” because it breaks Tailwind classes you might write: usually you can write text-white for example, but if you set the full palette, you’d have to use text-white-50 for the brightest shade, text-white would no longer be valid.

This kind of a conceptual distinction: there are palettes that range from dark to light (gray, indigo, cyan, etc.) and then there are individual named colours that don’t make sense with varying luminosity (white, black, transparent). If we take text-white-900 as an example (assuming white: colors.stone), then the resulting value is #1c1917. If you ask people what shade or colour #1c1917 is, “white” would be an unlikely answer, so that’s why usually you’d use gray: colors.stone and work with light and dark grays, not light and dark whites.

@Fryuni
Copy link
Member Author

Fryuni commented Apr 6, 2024

Interesting! In that case I think this should be an error at Tailwind level. Is this something I should report on Astro's Tailwind integration or on Tailwind directly? I have no idea how CSS tooling works and where each one's responsibility ends.

@delucis
Copy link
Member

delucis commented Apr 8, 2024

I think this should be an error at Tailwind level

I still think this is a Starlight-specific limitation that should have a nice error on our end rather than anything else. While Tailwind has that shape I described as a sensible default, it’s also in the end agnostic about what users decide to do with it. For example, Tailwind uses that <color>-<number> format for their shades, but it’s totally valid to provide a config shape that is completely different:

colors: {
  rainbow: {
    red: '#ff0000',
    orange: '#ffa500',
    yellow: '#ffff00',
    // ...
  },
},

And then use those as class="text-rainbow-red". Most people use Tailwind and only slightly tweak any of its defaults, but it also works as an entirely custom utility class generator (there’s an example a bit like this in this “A CSS project boilerplate” article).

So I don’t think Tailwind would see this as an error on their end, and the Astro integration does very close to nothing, so it’s not there either.

Instead I see it as:

  • As a generic tool, Tailwind has sensible defaults but is 100% customisable.
  • As an opinionated tool, Starlight makes some assumptions about Tailwind config shape and expects white to be a literal value. (Same way it expects to use gray for its gray shades and invents an “accent” palette to use for accent colours.)

If we were imposing a limitation that was unacceptable, we would aim to fix that and make our plugin more flexible. But in this case, I think expecting white to be a single colour, not a range of colours shouldn’t realistically cause problems for users. So instead we should make sure error messages and docs are more helpful on this point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants