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

Deduplication swallows variable when name and value are identical (custom PostCSS plugin) #11473

Closed
peterhirn opened this issue Jun 22, 2023 · 3 comments

Comments

@peterhirn
Copy link

What version of Tailwind CSS are you using?

v3.3.2

What build tool (or framework if it abstracts the build tool) are you using?

postcss 8.4.24, vite 4.3.9

What version of Node.js are you using?

v20.3.0

What browser are you using?

Chrome

What operating system are you using?

Windows 10

Reproduction

tailwind.config.js

/** @type {import('tailwindcss').Config} */
export default {
  content: ["./index.html", "./src/**/*.{js,ts,jsx,tsx}"],
  plugins: []
}

postcss.config.js

import { basename } from "node:path"
import tailwindcss from "tailwindcss"
import autoprefixer from "autoprefixer"

const appendMode = () => ({
  postcssPlugin: "variables-append-mode",
  Declaration(decl) {
    if (
      decl.variable &&
      decl.prop.startsWith("--md-") &&
      !decl.prop.endsWith("-light") &&
      !decl.prop.endsWith("-dark")
    ) {
      switch (basename(decl.source.input.file)) {
        case "tokens.light.css":
          decl.prop = `${decl.prop}-light`
          break

        case "tokens.dark.css":
          decl.prop = `${decl.prop}-dark`
          break
      }
    }
  }
})

appendMode.postcss = true

export default {
  plugins: [appendMode, tailwindcss, autoprefixer]
}

tokens.light.css

:root,
:host {
  --md-sys-color-primary: #005079;
  --md-sys-color-secondary: #4b6174;
  --md-sys-color-outline: #6e757d;
}

tokens.dark.css

:root,
:host {
  --md-sys-color-primary: #8fcdff;
  --md-sys-color-secondary: #b2c9df;
  --md-sys-color-outline: #6e757d;
}

index.css

@import "./tokens.light.css";
@import "./tokens.dark.css";

@tailwind base;
@tailwind components;
@tailwind utilities;

Describe your issue

I have a set of two css files which declare variables with identical variable names (tokens.light.css and tokens.dark.css). I'm using this minimal PostCSS plugin to automatically append -light/-dark to the variable names depending on which file they were declared in. Additionally I also convert the hex colors to rgb but this is not relevant here.

This works perfectly until a variable has not only the same name but also the same value, see --md-sys-color-outline above. In this case one of the variants is not created, here --md-sys-color-outline-light is missing.

Expected

:root, :host {
    --md-sys-color-primary-light: #005079;
    --md-sys-color-secondary-light: #4b6174;
    --md-sys-color-outline-light: #6e757d;
    --md-sys-color-primary-dark: #8fcdff;
    --md-sys-color-secondary-dark: #b2c9df;
    --md-sys-color-outline-dark: #6e757d;
}

Actual

:root, :host {
    --md-sys-color-primary-light: #005079;
    --md-sys-color-secondary-light: #4b6174;
    --md-sys-color-primary-dark: #8fcdff;
    --md-sys-color-secondary-dark: #b2c9df;
    --md-sys-color-outline-dark: #6e757d;
}
@joeypohie

This comment was marked as spam.

@thecrypticace
Copy link
Contributor

Hey! Rules with identical selectors are merged and identical properties are removed. I doubt this is a behavior we'll remove anytime soon as we rely on it for some things. I would suggest wrapping your imported files in native CSS layers:

@import "./tokens.light.css" layer(light);
@import "./tokens.dark.css" layer(dark);

This should prevent them from being merged and de-duplicated (because the at-rules will have different "names"). You can then look for @layer light and @layer dark in your PostCSS plugin and remove the wrapping layer by replacing it with the child nodes:

This is pseudo code for a PostCSS visitor so it may need some tweaks but the general idea is this:

AtRule(rule) {
  if (rule.name === 'layer' && (rule.params === 'dark' || rule.params === 'light')) {
    rule.replaceWith(...rule.nodes)
  }
},

Note that you'll want to be sure to run this after Tailwind CSS is run.

Hope that helps! ✨

@peterhirn
Copy link
Author

Hey! Thanks for your response.

I just realized that my approach was nonsensical. I ended up with all my classes looking like "bg-primary-light dark:bg-primary-dark" which is noisy and redundant.

I now wrapped the token.css files in html[data-mode="dark"] / html[data-mode="light"], adjusted my tailwind config and removed all dark: variants and -light suffixes. So the example above is now simply "bg-primary" ¯\_(ツ)_/¯

I'll dump my full config below for anyone interested. Variables / token files were generated using https://material-components.github.io/material-web/.

tokens.light.css

html[data-mode="light"] {
  --md-sys-color-background: #f8f9fd;
  --md-sys-color-on-background: #191c1f;
  --md-sys-color-surface: #f8f9fd;
  ...
}

tokens.dark.css

html[data-mode="dark"] {
  --md-sys-color-background: #111417;
  --md-sys-color-on-background: #e1e2e6;
  --md-sys-color-surface: #111417;
  ...
}

index.css

@import "./tokens.light.css";
@import "./tokens.dark.css";

@tailwind base;
@tailwind components;
@tailwind utilities;

postcss.config.js

import tailwindcss from "tailwindcss"
import autoprefixer from "autoprefixer"

const variablesToRgb = () => ({
  postcssPlugin: "material-variables-to-rgb",
  Declaration(decl) {
    if (
      decl.variable &&
      decl.prop.startsWith("--md-") &&
      decl.value.startsWith("#") &&
      decl.value.length === 7
    ) {
      const color = parseInt(decl.value.slice(1, 7), 16)
      const r = (color >> 16) & 255
      const g = (color >> 8) & 255
      const b = color & 255
      decl.value = `${r} ${g} ${b}`
    }
  }
})

variablesToRgb.postcss = true

export default {
  plugins: [variablesToRgb, tailwindcss, autoprefixer]
}

tailwind.config.js

/** @type {import('tailwindcss').Config} */
export default {
  darkMode: ["class", '[data-mode="dark"]'],
  content: ["./index.html", "./src/**/*.{js,ts,jsx,tsx}"],
  future: {
    hoverOnlyWhenSupported: true
  },
  theme: {
    extend: {
      colors: {
        primary: {
          DEFAULT: "rgb(var(--md-sys-color-primary) / <alpha-value>)",
          container: "rgb(var(--md-sys-color-primary-container) / <alpha-value>)"
        },
        secondary: {
          DEFAULT: "rgb(var(--md-sys-color-secondary) / <alpha-value>)",
          container: "rgb(var(--md-sys-color-secondary-container) / <alpha-value>)"
        },
        tertiary: {
          DEFAULT: "rgb(var(--md-sys-color-tertiary) / <alpha-value>)",
          container: "rgb(var(--md-sys-color-tertiary-container) / <alpha-value>)"
        },
        error: {
          DEFAULT: "rgb(var(--md-sys-color-error) / <alpha-value>)",
          container: "rgb(var(--md-sys-color-error-container) / <alpha-value>)"
        },
        surface: {
          DEFAULT: "rgb(var(--md-sys-color-surface) / <alpha-value>)",
          dim: "rgb(var(--md-sys-color-surface-dim) / <alpha-value>)",
          bright: "rgb(var(--md-sys-color-surface-bright) / <alpha-value>)",
          tint: "rgb(var(--md-sys-color-surface-tint) / <alpha-value>)",
          variant: "rgb(var(--md-sys-color-surface-variant) / <alpha-value>)",
          container: {
            DEFAULT: "rgb(var(--md-sys-color-surface-container) / <alpha-value>)",
            lowest: "rgb(var(--md-sys-color-surface-container-lowest) / <alpha-value>)",
            low: "rgb(var(--md-sys-color-surface-container-low) / <alpha-value>)",
            high: "rgb(var(--md-sys-color-surface-container-high) / <alpha-value>)",
            highest: "rgb(var(--md-sys-color-surface-container-highest) / <alpha-value>)"
          }
        },
        inverse: {
          surface: "rgb(var(--md-sys-color-inverse-surface) / <alpha-value>)",
          primary: "rgb(var(--md-sys-color-inverse-primary) / <alpha-value>)",
          on: {
            surface: "rgb(var(--md-sys-color-inverse-on-surface) / <alpha-value>)",
            primary: "rgb(var(--md-sys-color-inverse-on-primary) / <alpha-value>)"
          }
        },
        background: "rgb(var(--md-sys-color-background) / <alpha-value>)",
        outline: {
          DEFAULT: "rgb(var(--md-sys-color-outline) / <alpha-value>)",
          variant: "rgb(var(--md-sys-color-outline-variant) / <alpha-value>)"
        },
        shadow: "rgb(var(--md-sys-color-shadow) / <alpha-value>)",
        scrim: "rgb(var(--md-sys-color-scrim) / <alpha-value>)",
        on: {
          primary: {
            DEFAULT: "rgb(var(--md-sys-color-on-primary) / <alpha-value>)",
            container: "rgb(var(--md-sys-color-on-primary-container) / <alpha-value>)"
          },
          secondary: {
            DEFAULT: "rgb(var(--md-sys-color-on-secondary) / <alpha-value>)",
            container: "rgb(var(--md-sys-color-on-secondary-container) / <alpha-value>)"
          },
          tertiary: {
            DEFAULT: "rgb(var(--md-sys-color-on-tertiary) / <alpha-value>)",
            container: "rgb(var(--md-sys-color-on-tertiary-container) / <alpha-value>)"
          },
          error: {
            DEFAULT: "rgb(var(--md-sys-color-on-error) / <alpha-value>)",
            container: "rgb(var(--md-sys-color-on-error-container) / <alpha-value>)"
          },
          surface: {
            DEFAULT: "rgb(var(--md-sys-color-on-surface) / <alpha-value>)",
            variant: "rgb(var(--md-sys-color-on-surface-variant) / <alpha-value>)"
          },
          background: "rgb(var(--md-sys-color-on-background) / <alpha-value>)"
        }
      },
    }
  },
  plugins: []
}

KevinMulhern added a commit to TheOdinProject/theodinproject that referenced this issue Jul 10, 2023
Because:
* The dependabot bump was previously failing because of an import duplication error.

This commit:
* Remove our layout gradient class, it was causing the build to fail because it [duplicates an import](tailwindlabs/tailwindcss#11473). We were only using this class in one place so I just moved the styles to the element.
* Bump to latest version of Tailwind.
KevinMulhern added a commit to TheOdinProject/theodinproject that referenced this issue Jul 10, 2023
Because:
* The dependabot bump was previously failing because of an import
duplication error.

This commit:
* Remove our layout gradient class, it was causing the build to fail
because it [duplicates an
import](tailwindlabs/tailwindcss#11473). We
were only using this class in one place so I just moved the styles to
the element.
* Bump to latest version of Tailwind.
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

No branches or pull requests

3 participants