fix: nuxt module tailwind support #1033
Conversation
🦋 Changeset detectedLatest commit: f913eb5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
Warning Rate limit exceeded@juliusmarminge has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 34 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes introduce a dynamic approach to style inclusion in a Nuxt application, specifically determining whether to add a stylesheet or a Tailwind plugin based on the presence of the Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
A new canary is available for testing. You can install this latest build in your project with: pnpm add @uploadthing/[email protected]
pnpm add @uploadthing/[email protected]
pnpm add @uploadthing/[email protected]
pnpm add @uploadthing/[email protected]
pnpm add @uploadthing/[email protected]
pnpm add @uploadthing/[email protected]
pnpm add @uploadthing/[email protected]
pnpm add [email protected]
pnpm add @uploadthing/[email protected] |
📦 Bundle size comparison
|
More templates
commit: |
vercel
bot
temporarily deployed
to
Preview – legacy-docs-uploadthing
vercel
bot
temporarily deployed
to
Preview – legacy-docs-uploadthing
vercel
bot
temporarily deployed
to
Preview – legacy-docs-uploadthing
vercel
bot
temporarily deployed
to
Preview – legacy-docs-uploadthing
|
Hey @danielroe would you mind checking if this adheres to conventions or if I'm doing something silly here :) Thanks! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (6)
.changeset/little-boxes-taste.md (1)
1-6: Enhance the changeset description with more details.The patch version bumps are appropriate for this backwards-compatible fix. However, the description could be more detailed to help users understand the impact and any required actions.
Consider expanding the description like this:
--- "uploadthing": patch "@uploadthing/nuxt": patch --- -fix: dynamically add either stylesheet or tailwind plugin based on whether `@nuxtjs/tailwindcss´ is installed +fix: dynamically add either stylesheet or tailwind plugin based on whether `@nuxtjs/tailwindcss` is installed + +This change improves the Nuxt module's compatibility with Tailwind CSS by: +- Automatically detecting if `@nuxtjs/tailwindcss` is installed +- Injecting appropriate styles based on the setup +- No manual configuration required from userspackages/nuxt/playground/package.json (1)
12-12: Consider adding a comment about the dependency's purpose.To improve maintainability, consider adding a comment explaining that this dependency is required for testing the Tailwind CSS integration.
{ "devDependencies": { + // Required for testing Tailwind CSS integration "@nuxtjs/tailwindcss": "^6.12.2", "nuxt": "^3.11.2" } }packages/nuxt/package.json (1)
39-39: LGTM! The dependency addition aligns with the PR objectives.The addition of
@nuxtjs/tailwindcssas a devDependency is necessary for playground resolution, as previously noted. This change supports the implementation of dynamic Tailwind CSS support in the Nuxt module.Consider adding a comment in the package.json to document why this is needed as a devDependency:
{ "devDependencies": { + // Required for playground resolution "@nuxtjs/tailwindcss": "^6.12.2",packages/uploadthing/src/tw.ts (3)
10-30: Documentation could be enhanced with examples.The plugin implementation looks good and follows best practices. The documentation is clear but could be more helpful with usage examples.
Consider adding code examples to the documentation to show how to use these variants:
/** * UploadThing Tailwind plugin which injects custom variants * for the built-in UI components * @see https://docs.uploadthing.com/concepts/theming#theming-with-tailwind-css * * When using this, you need to specify `content` manually. For automatic * detection, see {@link withUt}. + * + * @example + * // Using element variants + * <div class="ut-button:bg-blue-500 ut-label:text-gray-700"> + * + * // Using state variants + * <div class="ut-uploading:bg-yellow-500 ut-ready:bg-green-500"> */
32-37: Enhance function documentation with parameter and return details.The documentation could be more detailed about the function's behavior.
Consider updating the documentation:
/** * HOF for Tailwind config that adds the * {@link uploadthingPlugin} to the Tailwind config * as well as adds content paths to detect the necessary * classnames + * + * @param twConfig - The Tailwind configuration to enhance + * @returns The modified Tailwind configuration with UploadThing support + * @example + * // tailwind.config.js + * import { withUt } from "@uploadthing/react"; + * + * export default withUt({ + * // your tailwind config + * }); */
Line range hint
38-77: Consider improving error handling and configuration management.The implementation is functional but could be enhanced in several ways:
- The function mutates the input config directly, which could lead to unexpected side effects
- The warning message uses console.warn which might be suppressed in some environments
- The path resolution could be more resilient
Consider these improvements:
export function withUt(twConfig: Config) { + // Create a deep clone to avoid mutating the input + const config = JSON.parse(JSON.stringify(twConfig)); + const contentPaths = PACKAGES.map((pkg) => { try { const resolved = require.resolve(`@uploadthing/${pkg}`, { paths: [...module.paths, process.cwd()], }); + // Verify package.json exists to ensure we found the right package + require.resolve(`@uploadthing/${pkg}/package.json`); return dirname(resolved) + sep + "**"; } catch { return null; } }).filter((s) => s != null); if (contentPaths.length === 0) { - // eslint-disable-next-line no-console - console.warn(` + const warning = ` [uploadthing]: Unable to resolve path for uploadthing UI packages. As a workaround, you can manually add the paths to your content paths: - Find where your package manager has installed the distribution files, e.g. './node_modules/@uploadthing/react'. Note: If you have a monorepo, you may need to look up the tree to find the correct path. - Add the path to the 'content' field in your Tailwind configuration: content: [ // your other content paths './node_modules/@uploadthing/react/dist**' // <-- add this line ] - `); + `; + // Use process.emitWarning for better error handling + process.emitWarning(warning, { + type: 'UploadThingConfig', + code: 'UPLOADTHING_PATH_RESOLUTION' + }); } - if (Array.isArray(twConfig.content)) { - twConfig.content.push(...contentPaths); + if (Array.isArray(config.content)) { + config.content.push(...contentPaths); } else { // content can be an object too with `files` property - twConfig.content.files.push(...contentPaths); + config.content.files.push(...contentPaths); } - if (!twConfig.plugins) { - twConfig.plugins = []; + if (!config.plugins) { + config.plugins = []; } - twConfig.plugins.push(uploadthingPlugin); + config.plugins.push(uploadthingPlugin); - return twConfig; + return config; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
- .changeset/little-boxes-taste.md (1 hunks)
- packages/nuxt/package.json (1 hunks)
- packages/nuxt/playground/app.vue (1 hunks)
- packages/nuxt/playground/nuxt.config.ts (1 hunks)
- packages/nuxt/playground/package.json (1 hunks)
- packages/nuxt/src/module.ts (4 hunks)
- packages/uploadthing/src/tw.ts (2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/nuxt/playground/app.vue
🧰 Additional context used
🪛 Biome
packages/nuxt/src/module.ts
[error] 142-142: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (7)
packages/nuxt/playground/nuxt.config.ts (1)
4-4: LGTM! Verify Tailwind module installation.The addition of
@nuxtjs/tailwindcssfollows Nuxt's recommended approach for Tailwind CSS integration.Let's verify the module installation:
✅ Verification successful
Tailwind module is properly configured ✅
The verification confirms:
@nuxtjs/tailwindcssis correctly declared as a dependency with version ^6.12.2- No conflicting Tailwind configurations found in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if @nuxtjs/tailwindcss is properly declared as a dependency jq '.dependencies["@nuxtjs/tailwindcss"] // .devDependencies["@nuxtjs/tailwindcss"]' packages/nuxt/playground/package.json # Verify no conflicting Tailwind configurations exist fd tailwind.config.jsLength of output: 158
packages/nuxt/playground/package.json (1)
12-12: LGTM! Verify version consistency across packages.The addition of
@nuxtjs/tailwindcssas a devDependency is correct. However, let's ensure version consistency across the monorepo.✅ Verification successful
Version consistency verified across the monorepo ✅
The version of
@nuxtjs/tailwindcss(^6.12.2) is consistent across both instances found in the monorepo:
- packages/nuxt/package.json
- packages/nuxt/playground/package.json
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for version consistency of @nuxtjs/tailwindcss across all package.json files # Expected: All instances should use the same version (^6.12.2) echo "Checking @nuxtjs/tailwindcss versions across package.json files:" fd "package.json" | xargs grep -l "@nuxtjs/tailwindcss" | while read -r file; do echo "In $file:" jq '.dependencies["@nuxtjs/tailwindcss"] // .devDependencies["@nuxtjs/tailwindcss"]' "$file" doneLength of output: 660
packages/uploadthing/src/tw.ts (1)
Line range hint
1-77: Verify Nuxt module integration.Let's ensure this plugin is properly integrated with the Nuxt module mentioned in the PR objectives.
✅ Verification successful
Tailwind plugin is correctly integrated with Nuxt module
The verification shows proper integration:
- The Nuxt module correctly imports and uses the
uploadthingPluginfromuploadthing/tw- The plugin is properly added to the Tailwind configuration in the module
- Required dependencies are present (
@nuxtjs/tailwindcssanduploadthing)- The playground correctly includes both the UploadThing module and Tailwind module
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check how this plugin is integrated with Nuxt # Test 1: Find the Nuxt module configuration echo "Checking Nuxt module configuration..." rg -l "@nuxtjs/tailwindcss" packages/nuxt/ # Test 2: Find where this plugin is imported in Nuxt echo "Checking plugin usage in Nuxt..." rg -l "uploadthingPlugin" packages/nuxt/ # Test 3: Check for any Tailwind-related configurations echo "Checking Tailwind configurations..." fd -e js -e ts "tailwind.config" packages/nuxt/Length of output: 533
Script:
#!/bin/bash # Let's examine how the Nuxt module integrates with Tailwind # Check the module implementation echo "=== Nuxt Module Implementation ===" rg -A 10 "uploadthingPlugin" packages/nuxt/src/module.ts # Check Tailwind configuration in nuxt.config.ts echo -e "\n=== Nuxt Config Tailwind Setup ===" rg -A 5 "tailwindcss" packages/nuxt/playground/nuxt.config.ts # Check package.json dependencies echo -e "\n=== Package Dependencies ===" cat packages/nuxt/package.json | jq '.dependencies, .devDependencies'Length of output: 1359
packages/nuxt/src/module.ts (4)
2-2: Well-structured type definitions and documentation!The new
injectStylesoption is clearly documented with JSDoc comments, making the API self-documenting. The imports and type definitions are appropriate for the new functionality.Also applies to: 7-7, 10-10, 14-15, 23-30
43-43: LGTM: Sensible default configurationSetting
injectStyles: trueby default maintains backward compatibility while allowing users to opt-out if needed.
86-89: LGTM: Proper initialization orderThe style injection is correctly placed after component registration and respects the user's configuration through
options.injectStyles.
112-115: Verify Tailwind detection reliabilityThe current check for Tailwind might not catch all possible Tailwind setups.
✅ Verification successful
Let me check the package.json files to understand how Tailwind is being detected and configured across the repository, particularly in the Nuxt-related files.
Based on the results, I can now provide a final response:
Tailwind detection is reliable through
@nuxtjs/tailwindcssmoduleThe current implementation using
hasNuxtModule("@nuxtjs/tailwindcss")is actually a reliable approach because:
- It uses Nuxt's official kit utility
hasNuxtModuleto detect the module- The
@nuxtjs/tailwindcssmodule is the standardized way to use Tailwind in Nuxt applications- Both the main package and playground properly declare
@nuxtjs/tailwindcssas a devDependency- The module handles all the necessary Tailwind setup and configuration automatically
The check is appropriate for determining whether to include the standalone CSS or let Tailwind handle the styles.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for different Tailwind setup patterns in the codebase # Test 1: Check for direct Tailwind imports echo "Checking for direct Tailwind imports..." rg -l "tailwindcss|@tailwind" --type css # Test 2: Check for Tailwind config files echo "Checking for Tailwind config files..." fd "tailwind.config" # Test 3: Check for other Nuxt Tailwind modules echo "Checking for other Tailwind-related modules in package.json files..." fd "package.json" --exec rg -l "tailwindcss|@nuxtjs/tailwind"Length of output: 1728
Script:
#!/bin/bash # Check Nuxt-related package.json files for Tailwind configuration echo "Content of packages/nuxt/package.json:" cat packages/nuxt/package.json echo -e "\nContent of packages/nuxt/playground/package.json:" cat packages/nuxt/playground/package.json # Check the hasNuxtModule implementation echo -e "\nChecking hasNuxtModule implementation:" rg -A 5 "hasNuxtModule" packages/nuxt/src/Length of output: 3434
| const vueDist = await resolver.resolvePath("@uploadthing/vue"); | ||
| const contentPath = dirname(vueDist) + sep + "**"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use path.join for more robust path handling
The current path construction using string concatenation with sep could be improved.
-const contentPath = dirname(vueDist) + sep + "**";
+const contentPath = path.join(dirname(vueDist), "**");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const vueDist = await resolver.resolvePath("@uploadthing/vue"); | |
| const contentPath = dirname(vueDist) + sep + "**"; | |
| const vueDist = await resolver.resolvePath("@uploadthing/vue"); | |
| const contentPath = path.join(dirname(vueDist), "**"); |
| const template = addTemplate({ | ||
| filename: "uploadthing.tailwind.config.cjs", | ||
| write: true, | ||
| getContents: () => ` | ||
| const { uploadthingPlugin } = require('uploadthing/tw'); | ||
|
|
||
| module.exports = { | ||
| content: [ | ||
| ${JSON.stringify(contentPath)} | ||
| ], | ||
| plugins: [ | ||
| require('uploadthing/tw').uploadthingPlugin | ||
| ] | ||
| } | ||
| `, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting Tailwind config template
The inline template could be moved to a separate file for better maintainability.
Consider creating a separate template file (e.g., templates/tailwind.config.cjs.template) and using it like this:
+import { readFileSync } from 'node:fs';
+
const template = addTemplate({
filename: "uploadthing.tailwind.config.cjs",
write: true,
- getContents: () => `
- const { uploadthingPlugin } = require('uploadthing/tw');
-
- module.exports = {
- content: [
- ${JSON.stringify(contentPath)}
- ],
- plugins: [
- require('uploadthing/tw').uploadthingPlugin
- ]
- }
- `,
+ getContents: () => {
+ const templateContent = readFileSync(
+ resolver.resolve('./templates/tailwind.config.cjs.template'),
+ 'utf-8'
+ );
+ return templateContent.replace('{{contentPath}}', JSON.stringify(contentPath));
+ },
});Committable suggestion was skipped due to low confidence.
packages/nuxt/src/module.ts
Outdated
| // @ts-expect-error - Help pls | ||
| const twModuleOptions = (nuxt.options.tailwindcss ??= {}) as { |
There was a problem hiding this comment.
Fix type assertion to remove @ts-expect-error
The type assertion could be improved to properly type the Nuxt options.
-// @ts-expect-error - Help pls
-const twModuleOptions = (nuxt.options.tailwindcss ??= {}) as {
+interface TailwindOptions {
+ configPath?: string | string[];
+}
+const twModuleOptions = (nuxt.options.tailwindcss ??= {} as TailwindOptions);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // @ts-expect-error - Help pls | |
| const twModuleOptions = (nuxt.options.tailwindcss ??= {}) as { | |
| interface TailwindOptions { | |
| configPath?: string | string[]; | |
| } | |
| const twModuleOptions = (nuxt.options.tailwindcss ??= {} as TailwindOptions); |
🧰 Tools
🪛 Biome
[error] 142-142: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
juliusmarminge
force-pushed
the
nuxt-module-tailwind
branch
from
e79adb9 to
a7fe644
Compare
juliusmarminge
force-pushed
the
nuxt-module-tailwind
branch
from
a7fe644 to
f913eb5
Compare
Checks if the user is using tailwind and if they are, we inject our tailwind plugin instead of the stylesheet
Summary by CodeRabbit
Release Notes
New Features
@nuxtjs/tailwindcsspackage, enhancing configuration flexibility.Improvements
@nuxtjs/tailwindcssmodule.Chores
@nuxtjs/tailwindcssdependency to project configurations.