refactor: unify theme css imports#336
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
@farming-labs/astro
@farming-labs/astro-theme
@farming-labs/docs
@farming-labs/theme
@farming-labs/next
@farming-labs/nuxt
@farming-labs/nuxt-theme
@farming-labs/svelte
@farming-labs/svelte-theme
@farming-labs/tanstack-start
commit: |
Kinfe123
changed the title
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 45 files
Not reviewed (too large): packages/fumadocs/styles/bundles/shared-framework.css (~10,457 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="website/app/docs/installation/page.mdx">
<violation number="1" location="website/app/docs/installation/page.mdx:539">
P2: The docs tell users to pick a CSS import that "matches the theme in your config," but the config snippet uses `fumadocs()` while the CSS example uses `default/css`. There is no documented mapping between the `fumadocs()` theme factory and the `default/css` path, so a user could reasonably derive a non-existent path like `@farming-labs/theme/fumadocs/css`. Consider explicitly documenting that the default `fumadocs()` theme maps to `default/css` — or rephrasing the guidance so it doesn't imply the theme function name and CSS path are the same.</violation>
</file>
<file name="packages/docs/src/cli/templates.ts">
<violation number="1" location="packages/docs/src/cli/templates.ts:149">
P2: The `hasBuiltInThemeCssImport` helper's package-prefix and `/css` checks are evaluated independently, so any file that contains an unrelated local import with `/css` in its path (e.g., `./vendor/css/reset.css`) alongside any `@farming-labs/theme/` prefix will incorrectly skip the required theme CSS injection. Because this unified helper is now shared by all four framework injectors, the false-positive path affects every scaffolded framework. Consider matching the import as a single quoted path — for example with a regex like `@farming-labs\\/(?:theme|svelte-theme|astro-theme|nuxt-theme)\\/[^"'\\n]*\\/css` — so the package name and `/css` must be part of the same import specifier.</violation>
</file>
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
| ### 5. Import theme CSS in `src/app.css` | ||
|
|
||
| Add your theme's CSS to your global stylesheet so docs styling applies. Use the import that matches the theme in your config (e.g. `fumadocs` below; for `greentree` use `@farming-labs/svelte-theme/greentree/css`). | ||
| Add your theme's CSS to your global stylesheet so docs styling applies. Use the import that matches the theme in your config (e.g. `default` below; for `greentree` use `@farming-labs/theme/greentree/css`). |
There was a problem hiding this comment.
P2: The docs tell users to pick a CSS import that "matches the theme in your config," but the config snippet uses fumadocs() while the CSS example uses default/css. There is no documented mapping between the fumadocs() theme factory and the default/css path, so a user could reasonably derive a non-existent path like @farming-labs/theme/fumadocs/css. Consider explicitly documenting that the default fumadocs() theme maps to default/css — or rephrasing the guidance so it doesn't imply the theme function name and CSS path are the same.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At website/app/docs/installation/page.mdx, line 539:
<comment>The docs tell users to pick a CSS import that "matches the theme in your config," but the config snippet uses `fumadocs()` while the CSS example uses `default/css`. There is no documented mapping between the `fumadocs()` theme factory and the `default/css` path, so a user could reasonably derive a non-existent path like `@farming-labs/theme/fumadocs/css`. Consider explicitly documenting that the default `fumadocs()` theme maps to `default/css` — or rephrasing the guidance so it doesn't imply the theme function name and CSS path are the same.</comment>
<file context>
@@ -536,10 +536,10 @@ Replace `my-docs` with your project name. Then run `cd my-docs` and start the de
### 5. Import theme CSS in `src/app.css`
- Add your theme's CSS to your global stylesheet so docs styling applies. Use the import that matches the theme in your config (e.g. `fumadocs` below; for `greentree` use `@farming-labs/svelte-theme/greentree/css`).
+ Add your theme's CSS to your global stylesheet so docs styling applies. Use the import that matches the theme in your config (e.g. `default` below; for `greentree` use `@farming-labs/theme/greentree/css`).
```css title="src/app.css"
</file context>
| return `@farming-labs/theme/${getThemeInfo(theme).nextCssImport}/css`; | ||
| } | ||
|
|
||
| function hasBuiltInThemeCssImport(content: string): boolean { |
There was a problem hiding this comment.
P2: The hasBuiltInThemeCssImport helper's package-prefix and /css checks are evaluated independently, so any file that contains an unrelated local import with /css in its path (e.g., ./vendor/css/reset.css) alongside any @farming-labs/theme/ prefix will incorrectly skip the required theme CSS injection. Because this unified helper is now shared by all four framework injectors, the false-positive path affects every scaffolded framework. Consider matching the import as a single quoted path — for example with a regex like @farming-labs\\/(?:theme|svelte-theme|astro-theme|nuxt-theme)\\/[^"'\\n]*\\/css — so the package name and /css must be part of the same import specifier.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/docs/src/cli/templates.ts, line 149:
<comment>The `hasBuiltInThemeCssImport` helper's package-prefix and `/css` checks are evaluated independently, so any file that contains an unrelated local import with `/css` in its path (e.g., `./vendor/css/reset.css`) alongside any `@farming-labs/theme/` prefix will incorrectly skip the required theme CSS injection. Because this unified helper is now shared by all four framework injectors, the false-positive path affects every scaffolded framework. Consider matching the import as a single quoted path — for example with a regex like `@farming-labs\\/(?:theme|svelte-theme|astro-theme|nuxt-theme)\\/[^"'\\n]*\\/css` — so the package name and `/css` must be part of the same import specifier.</comment>
<file context>
@@ -168,16 +135,27 @@ const THEME_INFO: Record<string, ThemeInfo> = {
+ return `@farming-labs/theme/${getThemeInfo(theme).nextCssImport}/css`;
+}
+
+function hasBuiltInThemeCssImport(content: string): boolean {
+ return (
+ (content.includes("@farming-labs/theme/") ||
</file context>
| function hasBuiltInThemeCssImport(content: string): boolean { | |
| function hasBuiltInThemeCssImport(content: string): boolean { | |
| return /["']@farming-labs\/(?:theme|svelte-theme|astro-theme|nuxt-theme)\/[^"'\n]*\/css["']/.test( | |
| content, | |
| ); | |
| } |
|
Re-verified after opening the PR using the repo-pinned package manager (
Notes: plain |
|
Follow-up runnability check and fixes: Fixed:
Browser-verified dev servers with Playwright:
Build/quality after fixes:
|
|
Confirmed the hardline footer issue from the SvelteKit screenshot. The exact oversized nav did not reproduce in the current Next example because it uses different React/Fumadocs footer markup, but it did reproduce across the non-React hardline examples: SvelteKit, Astro, and Nuxt.\n\nPushed fix |
Kinfe123
added
the
refactor: theme css
|
Updated the PR with the SvelteKit Hardline parity fix. What changed:
Verified:
Notes: Nuxt/Astro/SvelteKit builds still print the existing gray-matter eval warning; Astro also notes local Node 23 while Vercel uses Node 24; Next prints the existing Turbopack NFT and |
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Not reviewed (too large): packages/fumadocs/styles/bundles/shared-framework.css (~34 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/fumadocs/styles/hardline.css">
<violation number="1" location="packages/fumadocs/styles/hardline.css:269">
P2: The Hardline theme's `.fd-docs-content h1–h4` headings lost their previous `text-transform: uppercase` declaration. This appears unrelated to the PR's stated goal of unifying CSS imports and scoping structural CSS to framework adapters. Uppercase remains in use for other Hardline elements (table headers, sidebar labels, nav cards), so removing it only from the primary content headings breaks internal consistency and is likely an accidental omission during the refactor. If intentional, it should be documented; otherwise the declaration should be restored.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| .fd-docs-content h3, | ||
| .fd-docs-content h4 { | ||
| text-transform: uppercase; | ||
| letter-spacing: 0.01em; |
There was a problem hiding this comment.
P2: The Hardline theme's .fd-docs-content h1–h4 headings lost their previous text-transform: uppercase declaration. This appears unrelated to the PR's stated goal of unifying CSS imports and scoping structural CSS to framework adapters. Uppercase remains in use for other Hardline elements (table headers, sidebar labels, nav cards), so removing it only from the primary content headings breaks internal consistency and is likely an accidental omission during the refactor. If intentional, it should be documented; otherwise the declaration should be restored.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/fumadocs/styles/hardline.css, line 269:
<comment>The Hardline theme's `.fd-docs-content h1–h4` headings lost their previous `text-transform: uppercase` declaration. This appears unrelated to the PR's stated goal of unifying CSS imports and scoping structural CSS to framework adapters. Uppercase remains in use for other Hardline elements (table headers, sidebar labels, nav cards), so removing it only from the primary content headings breaks internal consistency and is likely an accidental omission during the refactor. If intentional, it should be documented; otherwise the declaration should be restored.</comment>
<file context>
@@ -266,7 +266,6 @@ aside button[class*="bg-fd-secondary"] {
.fd-docs-content h3,
.fd-docs-content h4 {
- text-transform: uppercase;
letter-spacing: 0.01em;
}
</file context>
1 participant
Summary
@farming-labs/themeso every built-in theme can be imported through@farming-labs/theme/<theme>/cssacross Next.js, TanStack Start, SvelteKit, Astro, and Nuxt.data-fd-frameworkand scope the shared structural CSS to those adapters.Validation
pnpm install --lockfile-only --frozen-lockfilepnpm run format:checkpnpm testpnpm typecheckpnpm --filter @farming-labs/theme run buildpnpm --dir examples/sveltekit buildpnpm --dir examples/astro buildpnpm --dir examples/nuxt buildpnpm --dir examples/next buildpnpm --dir examples/tanstack-start buildpnpm run lintpasses with 0 errors and 10 existing warningsNotes
refactorGitHub label, so this uses the requestedrefactor:prefix in the commit and PR title.Summary by cubic
Unifies theme CSS imports across frameworks using a shared bundle in
@farming-labs/theme, so all themes load from@farming-labs/theme/<theme>/css. Also aligns Hardline heading/page‑nav styles across adapters and fixes dev configs.Refactors
@farming-labs/theme; themes importstyles/bundles/shared-framework.css.@farming-labs/theme/<theme>/cssimports for Next.js, TanStack Start, SvelteKit, Astro, and Nuxt.id="nd-docs-layout"+data-fd-framework(Astro, SvelteKit, Nuxt).@farming-labs/theme; updated examples, docs, and website.es2022and base body styles moved into layout; NuxtoptimizeDepsincludes@farming-labs/theme; safer Svelte tag injection to avoid preprocessor issues.Migration
@farming-labs/svelte-theme/<theme>/css→@farming-labs/theme/<theme>/css@farming-labs/astro-theme/<theme>/css→@farming-labs/theme/<theme>/csscss: ["@farming-labs/nuxt-theme/<theme>/css"]→css: ["@farming-labs/theme/<theme>/css"]@farming-labs/themeto deps for Astro, SvelteKit, and Nuxt.Written for commit af70678. Summary will update on new commits.