shadcn-ui / ui

Beautifully designed components that you can copy and paste into your apps. Accessible. Customizable. Open Source.
https://ui.shadcn.com
MIT License
74.1k stars 4.57k forks source link

[bug]: `globals.css` contains default NextJS CSS variables that interfere with the theme #4845

Open SDrinkwater opened 1 month ago

SDrinkwater commented 1 month ago

Describe the bug

Issue: After running npx shadcn@latest init -d, the resulting globals.css contains both the default NextJS setup as well as the themed css variables. These variables overlap and cause issues with the tailwind setup created via the shadcn command.

As a concrete example, the tailwind.config.ts background looks like this: background: "hsl(var(--background))" (note that it expects --background to be a hsl value. The problem is that the default NextJS setup uses hex for the themed colors. These override the ones specified by the shadcn theme and effectively corrupt the theme. The resulting style given to the browser looks like:

.bg-background {
    background-color: hsl(#ffffff);
}

Note that it is trying to use a hex value as an hsl value.

Since the value passed to the browser is not valid, it renders the background color as transparent which messes with all of the components that rely on this background theming (read: practically all of them).

I noticed this initially on the slider component as the thumb was entirely transparent.

Expected: The resulting globals.css file should not contain the default NextJS setup.

Workaround: If others face this issue and notice that their components look a little funny, simply remove the default NextJS theming (first :root block through to the first @layer utilities block

See the first part of the resulting globals.css file below:

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

:root {
  --background: #ffffff;
  --foreground: #171717;
}

@media (prefers-color-scheme: dark) {
  :root {
    --background: #0a0a0a;
    --foreground: #ededed;
  }
}

body {
  color: var(--foreground);
  background: var(--background);
  font-family: Arial, Helvetica, sans-serif;
}

@layer utilities {
  .text-balance {
    text-wrap: balance;
  }
}

@layer base {
  :root {
    --background: 0 0% 100%;
    --foreground: 0 0% 3.9%;

...

Affected component/components

All

How to reproduce

  1. Run npx shadcn@latest init -d
  2. Install slider npx shadcn@latest add slider
  3. Add slider to component and observe transparent slider thumb.

Codesandbox/StackBlitz link

No response

Logs

No response

System Info

Shadcn: 2.0.6

Before submitting

Lucas070713 commented 1 month ago

https://github.com/user-attachments/assets/d4925e6b-fc92-497f-b558-a5ef99f39f83

I am experiencing the same bug.

joseph0926 commented 1 month ago

https://github.com/shadcn-ui/ui/blob/9ef7967b0d39e7084f9e319aec0e97ca4a31a739/packages/shadcn/src/utils/updaters/update-css-vars.ts#L186


I think it's the logic that clears the “css” variable in the “next.js” in the updated “cli”. But looking at the logic, it doesn't seem to be effective in clearing the latest “next.js” “css” variable at the moment (v14.2.11)

I don't know if the following modification will work, but I'm open to suggestions and inquiries


function cleanupDefaultNextStylesPlugin() {
  return {
    postcssPlugin: "cleanup-default-next-styles",
    Once(root: Root) {
      // Remove specific variables from “:root” rules and delete rules  
      root.walkRules(":root", (rootRule) => {
        rootRule.walkDecls((decl) => {
          if (decl.prop === "--background" || decl.prop === "--foreground") {
            decl.remove()
          }
        })

        // If the “:root” rule is empty, remove it 
        if (rootRule.nodes.length === 0) {
          rootRule.remove()
        }
      })

      // Remove all declarations from the “body” rule and delete the rule 
      root.walkRules("body", (bodyRule) => {
        bodyRule.remove()
      })
    },
  }
}

https://github.com/shadcn-ui/ui/blob/main/packages/shadcn/test/utils/updaters/update-css-vars.test.ts

The test code for that utility is also not up to date with the latest “next.js”

Probably “next.js” broke the logic of “shadcn” when it updated the “css” variable

@shadcn

jiaweing commented 4 weeks ago

Same here, took a while to troubleshoot why the font was Arial and realised the culprit was in globals.css