feat: support browser runtime for tw plugin
#1072
Conversation
🦋 Changeset detectedLatest commit: 6a5265f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 10 minutes and 48 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 pull request includes updates to multiple Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
More templates
@uploadthing/expo
@uploadthing/nuxt
@uploadthing/mime-types
@uploadthing/shared
@uploadthing/react
uploadthing
@uploadthing/vue
commit: |
📦 Bundle size comparison
|
|
A new canary is available for testing. You can install this latest build in your project with: pnpm add @uploadthing/expo@7.1.2-canary.0fa10f9
pnpm add @uploadthing/mime-types@0.3.3-canary.0fa10f9
pnpm add @uploadthing/nuxt@7.1.2-canary.0fa10f9
pnpm add @uploadthing/react@7.1.2-canary.0fa10f9
pnpm add @uploadthing/shared@7.1.2-canary.0fa10f9
pnpm add @uploadthing/solid@7.1.2-canary.0fa10f9
pnpm add @uploadthing/svelte@7.1.2-canary.0fa10f9
pnpm add uploadthing@7.3.1-canary.0fa10f9
pnpm add @uploadthing/vue@7.1.2-canary.0fa10f9 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
packages/mime-types/package.json (1)
⚠️ Package exports configuration needs adjustmentThe package.json exports field references files in non-existent directories. The source files (application.ts, audio.ts, etc.) are located in the src directory, but the exports field incorrectly points to root-level directories. The exports paths should be updated to point to the dist directory where the built files will be located.
- Update the exports field paths to use
dist/prefix:"./application": { "import": { "types": "./dist/application/index.d.ts", "default": "./dist/application/index.js" } }- Similar updates needed for audio, image, text, and video exports
🔗 Analysis chain
Line range hint
9-67: Verify dual package hazard preventionThe exports field is well-structured for dual CJS/ESM support. However, let's verify that all entry points are properly built and that there are no dual package hazards.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all exported paths exist and match the expected format # Check if all the declared export paths exist for type in application audio image text video; do echo "Checking $type exports..." fd -t f "index\.(js|cjs|d\.ts|d\.cts)$" "$type" done # Check if package.json has proper "type": "module" field grep -r "require.*\.js\"" . && echo "Warning: Found .js extension in require conditions"Length of output: 1486
Script:
#!/bin/bash # Let's check if these directories exist in the dist folder instead echo "Checking dist directory structure..." fd -t d "^(application|audio|image|text|video)$" dist echo -e "\nChecking for entry point files in dist..." fd -t f "index\.(js|cjs|d\.ts|d\.cts)$" dist echo -e "\nChecking package.json type field..." grep '"type":' packages/mime-types/package.json echo -e "\nChecking build scripts in package.json..." grep -A 5 '"build":' packages/mime-types/package.jsonLength of output: 1054
Script:
#!/bin/bash # Let's check the source files and build configuration echo "Checking source files structure..." fd -t f "index\.ts$" src echo -e "\nChecking tsconfig.build.json contents..." cat packages/mime-types/tsconfig.build.json echo -e "\nChecking if bunchee is configured..." cat packages/mime-types/bunchee.config.ts 2>/dev/null || echo "No bunchee config found" echo -e "\nChecking source directory structure..." fd -t d "^(application|audio|image|text|video)$" srcLength of output: 863
Script:
#!/bin/bash # Let's check the correct paths within the package directory echo "Checking source files in package directory..." fd -t f "index\.ts$" packages/mime-types/src echo -e "\nChecking source directory structure in package..." ls -la packages/mime-types/src 2>/dev/null echo -e "\nChecking if dist directory exists in package..." ls -la packages/mime-types/dist 2>/dev/null echo -e "\nChecking package root structure..." ls -la packages/mime-types/Length of output: 1726
packages/uploadthing/src/tw/plugin.ts (1)
11-23: LGTM! Clean implementation of the Tailwind plugin.The plugin implementation follows best practices by using data attributes for variants and provides clear separation between element and state selectors.
Consider adding variants for additional upload states.
The current implementation covers basic states (
readying,ready,uploading), but common upload workflows might benefit from additional states:
ut-errorfor failed uploadsut-completedfor successful uploadsut-rejectedfor invalid file types// Variants to select specific state addVariant("ut-readying", '&[data-state="readying"]'); addVariant("ut-ready", '&[data-state="ready"]'); addVariant("ut-uploading", '&[data-state="uploading"]'); +addVariant("ut-error", '&[data-state="error"]'); +addVariant("ut-completed", '&[data-state="completed"]'); +addVariant("ut-rejected", '&[data-state="rejected"]');packages/uploadthing/src/tw/index.ts (1)
32-44: Enhance warning message clarityThe warning message could be more specific about the environment context.
Consider updating the message:
- [uploadthing]: Unable to resolve path for uploadthing UI packages. + [uploadthing]: Unable to resolve path for uploadthing UI packages. This is expected when running in a browser environment.
📜 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 (10)
packages/expo/package.json(1 hunks)packages/mime-types/package.json(1 hunks)packages/react/package.json(1 hunks)packages/shared/package.json(1 hunks)packages/uploadthing/package.json(2 hunks)packages/uploadthing/src/tw.ts(0 hunks)packages/uploadthing/src/tw/index.browser.ts(1 hunks)packages/uploadthing/src/tw/index.ts(1 hunks)packages/uploadthing/src/tw/plugin.ts(1 hunks)packages/vue/package.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/uploadthing/src/tw.ts
🔇 Additional comments (7)
packages/mime-types/package.json (1)
92-92: Verify the bunchee upgrade impact
The upgrade of bunchee from ^5.2.1 to ^5.6.2 might be related to the browser runtime support needed for the tw plugin. Let's verify the changelog for any breaking changes.
✅ Verification successful
Bunchee upgrade is safe and aligned with browser runtime support
The upgrade from 5.2.1 to 5.6.2 brings improvements that align well with the PR's objective of browser runtime support:
- v5.6.2: Improved handling of conditional imports for shared modules
- v5.6.1: Added support for browserlists from package.json
- v5.6.0: Mainly dependency updates
- v5.5.1: Enhanced support for special fields of target runtime
No breaking changes were introduced in this version range, and the changes actually improve browser-related functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check bunchee's changelog for breaking changes between 5.2.1 and 5.6.2
# Get the changelog or release notes
gh api repos/huozhi/bunchee/releases | jq -r '.[].body' | grep -A 10 "5\.6\."
Length of output: 1662
packages/shared/package.json (1)
46-46: Verify bunchee upgrade compatibility.
The upgrade from ^5.2.1 to ^5.6.2 is a minor version bump, but please verify:
- Any breaking changes in the bunchee changelog
- Compatibility with the browser bundle requirements for this PR
✅ Verification successful
Bunchee version upgrade is safe to proceed
The version upgrade from ^5.2.1 to ^5.6.2 appears safe as:
- Bunchee is consistently used across multiple packages in the monorepo with the same version ^5.6.2
- It's used primarily for building TypeScript/JavaScript packages with similar configuration patterns
- The minor version bump (5.2 to 5.6) follows semver, indicating backward compatibility
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check bunchee changelog for breaking changes between 5.2.1 and 5.6.2
# Fetch the changelog
curl -s https://raw.githubusercontent.com/huozhi/bunchee/main/CHANGELOG.md | grep -A 10 "5\.6\.2\|5\.6\.1\|5\.6\.0\|5\.5\.0\|5\.4\.0\|5\.3\.0"
Length of output: 144
Script:
#!/bin/bash
# Let's check the build configuration and usage of bunchee in the codebase
# Find files that mention bunchee
rg -l "bunchee" --type-add 'config:*.{json,js,ts,cjs,mjs}' -t config
# Check package.json files for bunchee version across packages
fd package.json --exec grep -l "bunchee" {} \; | xargs cat
# Look for bunchee configuration files
fd -e bunche.config.js -e bunchee.config.ts -e bunchee.config.mjs
Length of output: 19072
packages/uploadthing/src/tw/index.ts (1)
22-24:
Review Node.js-specific API usage
The require.resolve call with module.paths is Node.js specific and won't work in browsers. This needs a browser-compatible alternative.
Let's verify if there's a browser version of this file:
packages/expo/package.json (1)
43-43: Verify bunchee version compatibility
The upgrade to bunchee 5.6.2 might be related to browser bundle support.
Let's check the changelog for relevant changes:
✅ Verification successful
Let me gather more information about the specific changes in these versions.
Let me check the changes in intermediate versions to ensure we don't miss any significant browser-related changes.
Bunchee upgrade includes important browser support improvements
The upgrade from 5.2.1 to 5.6.2 includes significant browser-related enhancements:
- Support for browserlists from package.json (v5.6.0)
- Improved special fields handling for target runtime (v5.5.0)
- Enhanced ESM-shim matching (v5.6.2)
These changes are beneficial for browser bundle support and compatibility. The upgrade is appropriate and recommended.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Fetch bunchee releases between 5.2.1 and 5.6.2
gh release list -R huozhi/bunchee -L 10 | grep -E "v5\.[2-6]"
Length of output: 315
Script:
#!/bin/bash
# Fetch release notes for relevant versions
gh release view v5.6.2 -R huozhi/bunchee
gh release view v5.2.1 -R huozhi/bunchee
Length of output: 961
Script:
#!/bin/bash
# Check intermediate versions for browser-related changes
gh release view v5.6.0 -R huozhi/bunchee
gh release view v5.5.0 -R huozhi/bunchee
gh release view v5.4.0 -R huozhi/bunchee
gh release view v5.3.0 -R huozhi/bunchee
Length of output: 2552
packages/vue/package.json (1)
50-50: Verify consistent bunchee version
The bunchee version update matches other packages, maintaining consistency.
packages/react/package.json (1)
85-85: LGTM: Bunchee version bump as requested
The update to bunchee ^5.6.2 aligns with the requested change from the previous review and should help resolve the build issues.
packages/uploadthing/package.json (1)
169-169: LGTM: Bunchee version bump as requested
The update to bunchee ^5.6.2 aligns with the requested change from the previous review and should help resolve the build issues.
| if (Array.isArray(twConfig.content)) { | ||
| twConfig.content.push(...contentPaths); | ||
| } else { | ||
| // content can be an object too with `files` property | ||
| twConfig.content.files.push(...contentPaths); | ||
| } |
There was a problem hiding this comment.
Add type checking for content object structure
The code assumes the content object has a files property without type checking.
Add type checking to prevent runtime errors:
} else {
// content can be an object too with `files` property
+ if (!twConfig.content || !Array.isArray(twConfig.content.files)) {
+ twConfig.content = { files: [] };
+ }
twConfig.content.files.push(...contentPaths);
}📝 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.
| if (Array.isArray(twConfig.content)) { | |
| twConfig.content.push(...contentPaths); | |
| } else { | |
| // content can be an object too with `files` property | |
| twConfig.content.files.push(...contentPaths); | |
| } | |
| if (Array.isArray(twConfig.content)) { | |
| twConfig.content.push(...contentPaths); | |
| } else { | |
| // content can be an object too with `files` property | |
| if (!twConfig.content || !Array.isArray(twConfig.content.files)) { | |
| twConfig.content = { files: [] }; | |
| } | |
| twConfig.content.files.push(...contentPaths); | |
| } |
| @@ -0,0 +1,60 @@ | |||
| import { dirname, sep } from "node:path"; | |||
There was a problem hiding this comment.
Consider browser compatibility for Node.js imports
The node:path import will cause issues in browser environments. Given the PR's objective to support browser runtime, this needs to be addressed.
Consider creating a browser-specific version without Node.js dependencies:
- import { dirname, sep } from "node:path";
+ // For browser version:
+ const sep = '/';
+ const dirname = (path: string) => path.substring(0, path.lastIndexOf(sep));Committable suggestion skipped: line range outside the PR's diff.
|
A new canary is available for testing. You can install this latest build in your project with: pnpm add @uploadthing/expo@7.1.2-canary.2352c0f
pnpm add @uploadthing/mime-types@0.3.3-canary.2352c0f
pnpm add @uploadthing/nuxt@7.1.2-canary.2352c0f
pnpm add @uploadthing/react@7.1.2-canary.2352c0f
pnpm add @uploadthing/shared@7.1.2-canary.2352c0f
pnpm add @uploadthing/solid@7.1.2-canary.2352c0f
pnpm add @uploadthing/svelte@7.1.2-canary.2352c0f
pnpm add uploadthing@7.3.1-canary.2352c0f
pnpm add @uploadthing/vue@7.1.2-canary.2352c0f |
Closes #1070
adds a version of the /tw subpath that doesn't use node so you can import your tailwind config in the browser
Summary by CodeRabbit
Release Notes
New Features
twmodule.Improvements
buncheedependency across multiple packages to enhance build tool performance and compatibility.Bug Fixes
Documentation