feat(theme): allow importing @nuxt/ui into a cascade layer#6602
feat(theme): allow importing @nuxt/ui into a cascade layer#6602pupuking723 wants to merge 1 commit into
@nuxt/ui into a cascade layer#6602Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR splits Nuxt UI source directives into a dedicated runtime sources stylesheet so CSS imports can be layered without nested Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/templates.ts (1)
386-390:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate the watched template target for component detection.
After moving generated
@sourcedirectives toui.sources.css, the watcher still refreshes onlyui.css. In dev, component-detection changes won’t regenerate sources, so Tailwind scanning can go stale until restart.Suggested fix
- await updateTemplates({ filter: template => template.filename === 'ui.css' }) + await updateTemplates({ + filter: template => template.filename === 'ui.sources.css' + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/templates.ts` around lines 386 - 390, In the builder:watch hook within the component detection block, the updateTemplates function is filtering for the wrong template filename. Since the generated `@source` directives have been moved to ui.sources.css, update the template filter condition from checking for ui.css to ui.sources.css so that component detection changes in dev mode properly regenerate the sources file and keep Tailwind scanning current.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/unplugin.ts`:
- Around line 101-107: The transform function in the unplugin.ts file currently
prepends the `@import` statement at the very beginning of the code, which violates
CSS specifications when the file starts with a `@charset` rule since `@charset` must
be the absolute first declaration. Modify the transform function to detect if
the code begins with a `@charset` rule and if so, insert the `@import` statement
after the `@charset` declaration instead of at byte 0. For files without `@charset`,
continue prepending the `@import` at the beginning as currently implemented.
In `@test/utils/css-layer.spec.ts`:
- Around line 1-9: The test file is missing the necessary imports for Vitest
global functions (describe, it, expect) which are used throughout the file
starting at line 28-42. Although vitest.config.ts enables globals: true,
TypeScript requires explicit imports for type checking. Add an import statement
from 'vitest' that imports describe, it, and expect at the beginning of the
file, following the pattern used in other test files in the repository.
---
Outside diff comments:
In `@src/templates.ts`:
- Around line 386-390: In the builder:watch hook within the component detection
block, the updateTemplates function is filtering for the wrong template
filename. Since the generated `@source` directives have been moved to
ui.sources.css, update the template filter condition from checking for ui.css to
ui.sources.css so that component detection changes in dev mode properly
regenerate the sources file and keep Tailwind scanning current.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5aafbfec-2fcb-4098-9ab6-03e1c8e37b7f
📒 Files selected for processing (7)
package.jsonsrc/module.tssrc/runtime/index.csssrc/runtime/sources.csssrc/templates.tssrc/unplugin.tstest/utils/css-layer.spec.ts
💤 Files with no reviewable changes (1)
- src/runtime/index.css
71b132e to
61c0e4a
Compare
61c0e4a to
4cd0a59
Compare
commit: |
4cd0a59 to
724fda8
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/utils/css.ts (1)
14-14: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueMinor: CRLF files leave a stray carriage return after
@charset.
code.slice(charset.length)yields\r\n@import …on CRLF stylesheets, but.replace(/^\n/, '')only strips a leading\n, so the\r(and an extra blank line) survives. Harmless for parsing but slightly messy output.♻️ Handle CRLF too
- return `${charset}\n${uiSourcesImport}${code.slice(charset.length).replace(/^\n/, '')}` + return `${charset}\n${uiSourcesImport}${code.slice(charset.length).replace(/^\r?\n/, '')}`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/css.ts` at line 14, The string assembly in css.ts leaves a stray carriage return for CRLF stylesheets because `code.slice(charset.length).replace(/^\n/, '')` only removes a leading LF. Update the logic in the charset handling path to strip an initial CRLF pair as well as LF so the result from `charset` and `uiSourcesImport` is clean on both newline styles.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/utils/css.ts`:
- Line 14: The string assembly in css.ts leaves a stray carriage return for CRLF
stylesheets because `code.slice(charset.length).replace(/^\n/, '')` only removes
a leading LF. Update the logic in the charset handling path to strip an initial
CRLF pair as well as LF so the result from `charset` and `uiSourcesImport` is
clean on both newline styles.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1d73a57e-f50b-44e8-827f-d2525c1dbd7e
📒 Files selected for processing (9)
docs/app/pages/docs/[...slug].vuepackage.jsonsrc/module.tssrc/runtime/index.csssrc/runtime/sources.csssrc/templates.tssrc/unplugin.tssrc/utils/css.tstest/utils/css-layer.spec.ts
💤 Files with no reviewable changes (1)
- src/runtime/index.css
✅ Files skipped from review due to trivial changes (1)
- src/module.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/templates.ts
- docs/app/pages/docs/[...slug].vue
- src/runtime/sources.css
- package.json
724fda8 to
865d72e
Compare
865d72e to
a64cdec
Compare
@nuxt/ui into a cascade layer
benjamincanac
left a comment
There was a problem hiding this comment.
Thanks @pupuking723, the @nuxt/ui/sources split is the right idea but there's a blocker on the Nuxt side.
Loading sources.css via nuxt.options.css.unshift() makes it a separate Vite module, and with @tailwindcss/vite a file is only a Tailwind root if it has @import "tailwindcss". sources.css doesn't, so its @source/@variant are never processed. I confirmed with a playgrounds/nuxt build: dark: utilities fall back to @media (prefers-color-scheme) (class toggle dead), @variant shows up as raw text, and light: utilities aren't generated.
The Vue path works only because the import gets injected into the user's main.css, which is the real root. The directives have to live in the same root as @import "tailwindcss", so either document one line in main.css outside any layer, or run the injection for Nuxt too and drop the css.unshift. Either way please add the Nuxt two root case to the test, since the current one compiles everything in a single root and stays green while Nuxt is broken.
5a02695 to
1544c42
Compare
|
Updated this to address the Nuxt two-root issue you pointed out. Changes:
Verification after rebasing onto the latest
|
|
Thanks @pupuking723! Been thinking about this one and wanted to float an idea before you go further. What if instead of injecting The idea, fully non breaking: keep @import "tailwindcss";
@import "@nuxt/ui/sources";
@import "@nuxt/ui/base" layer(tailwindcss);That would drop |
Fixes #4503.
This moves Tailwind-only directives out of the layerable Nuxt UI stylesheet so users can import Nuxt UI into a CSS cascade layer:
Changes:
@sourcedirectives into@nuxt/ui/sources@variant light/darkoutside the layerable@nuxt/uistylesheet@nuxt/uiVerification:
pnpm exec eslint src/unplugin.ts src/module.ts src/templates.ts test/utils/css-layer.spec.tspnpm exec vitest run test/utils/css-layer.spec.ts --project vuepnpm build