fix: nuxt module options #1056
fix: nuxt module options #1056
Conversation
🦋 Changeset detectedLatest commit: e7484ab The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 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
|
WalkthroughThis pull request introduces several updates primarily focused on version management across multiple packages, including Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
commit: |
📦 Bundle size comparison
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (6)
.changeset/six-dodos-brush.md (1)
1-5: Enhance the changeset description with more details.While the changeset correctly indicates this is a patch release, the description could be more helpful for future reference by including:
- Which specific module options were affected
- How the static analysis issue manifested
- Whether this relates to the environment variable prefixing (
NUXT_) mentioned in issue #1047- What changes were made to fix the issue
Consider expanding the description to something like:
--- "@uploadthing/nuxt": patch --- -fix module options not honored due to static analysis +fix: ensure Nuxt module correctly honors environment variable prefixes + +Previously, the module didn't properly handle the `NUXT_` prefix for environment +variables (e.g., NUXT_UPLOADTHING_TOKEN), causing confusion with documentation. +This patch implements proper static analysis compatibility for module options, +ensuring environment variables work as documented.packages/nuxt/src/runtime/server/api/uploadthing.ts (3)
8-15: Consider enhancing the utility function implementationWhile the function works correctly, it could be more robust and concise.
Consider this improved implementation:
-const emptyStringToUndefined = (obj: Record<string, unknown>) => { - const newObj: Record<string, unknown> = {}; - for (const key in obj) { - if (obj[key] === "") newObj[key] = undefined; - else newObj[key] = obj[key]; - } - return newObj; -}; +const emptyStringToUndefined = (obj: Record<string, unknown>) => { + if (!obj || typeof obj !== 'object') return {}; + return Object.fromEntries( + Object.entries(obj).map(([key, value]) => + [key, value === "" ? undefined : value] + ) + ); +};This version:
- Adds input validation
- Uses functional methods for better readability
- Maintains the same type safety
18-19: Consider using a more specific type for runtime configThe current type assertion is quite loose. Consider defining a more specific interface for the uploadthing config.
interface UploadthingRuntimeConfig { uploadthing?: RouteHandlerConfig; } const runtime = useRuntimeConfig() as UploadthingRuntimeConfig;
21-21: Remove debug console.log statementThis appears to be debugging code that shouldn't be in production.
- console.log(config);packages/nuxt/src/module.ts (2)
Line range hint
19-33: Document the newly added configuration options in ModuleOptions type.The type definition should include documentation for all available configuration options, including the newly added ones (
token,isDev,logLevel, etc.) to improve developer experience and type safety.Consider adding these properties with JSDoc comments:
export type ModuleOptions = RouteHandlerConfig & { routerPath: string; /** * Injects UploadThing styles into the page * If you're using Tailwind, it will inject the * UploadThing Tailwind plugin instead. * * @default true */ injectStyles: boolean; + /** + * Authentication token for UploadThing + * Can be set via UPLOADTHING_TOKEN environment variable + */ + token?: string; + /** + * Development mode flag + */ + isDev?: boolean; + /** + * Logging level configuration + */ + logLevel?: string; + /** + * Logging format configuration + */ + logFormat?: string; + /** + * Custom ingest URL for UploadThing + */ + ingestUrl?: string; + /** + * Custom callback URL for UploadThing + */ + callbackUrl?: string; };
Line range hint
58-64: Consider enhancing runtime configuration handling.The current implementation merges module options into runtime config, but there might be room for improvement in environment variable handling. Consider:
- Adding validation for environment variables
- Implementing a clear precedence order (env vars > runtime config > defaults)
- Documenting the configuration resolution process
This would help address the environment variable naming confusion mentioned in issue #1047.
📜 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 (11)
.changeset/healthy-experts-exist.md(1 hunks).changeset/six-dodos-brush.md(1 hunks)examples/backend-adapters/server/package.json(1 hunks)package.json(1 hunks)packages/nuxt/src/module.ts(1 hunks)packages/nuxt/src/runtime/server/api/uploadthing.ts(1 hunks)packages/shared/package.json(1 hunks)packages/uploadthing/package.json(1 hunks)packages/uploadthing/src/sdk/index.ts(0 hunks)playground-v6/package.json(1 hunks)playground/package.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/uploadthing/src/sdk/index.ts
✅ Files skipped from review due to trivial changes (3)
- .changeset/healthy-experts-exist.md
- package.json
- playground/package.json
🔇 Additional comments (8)
playground-v6/package.json (2)
14-14: LGTM: Effect package version update
The update of effect from 3.10.3 to 3.10.15 aligns with the version standardization across other packages in the repository.
Line range hint 15-18: Verify stability of dependency versions
There are potential stability concerns:
- Using Next.js
canaryversion in a playground environment could lead to unexpected behavior - React version 18.3.1 is ahead of the current stable release
Consider:
- Pinning Next.js to a stable version for more predictable behavior
- Verifying compatibility between React 18.3.1 and the uploadthing v6 package
packages/nuxt/src/runtime/server/api/uploadthing.ts (1)
18-26: Verify environment variable handling
Based on the linked issue #1047, there seems to be confusion about environment variable naming (UPLOADTHING_TOKEN vs NUXT_UPLOADTHING_TOKEN). This change handles empty strings, but we should verify if it properly handles both environment variable formats.
✅ Verification successful
The initial search revealed multiple documentation files, but we need to specifically check the Nuxt integration documentation and the runtime configuration handling to verify the environment variable naming convention. Let's examine these to determine if there's clear documentation about which environment variable format should be used.
Environment variable naming convention is correctly documented
The documentation explicitly states that the NUXT_ prefix is important for the environment variable, with a clear example:
NUXT_UPLOADTHING_TOKEN=... # A token for interacting with the SDK
# 👆 NUXT_ prefix is **important**The current implementation in uploadthing.ts that handles empty strings is consistent with this documentation, as it processes the configuration through Nuxt's useRuntimeConfig(), which automatically handles the NUXT_ prefixed environment variables. The confusion mentioned in issue #1047 is addressed by the clear documentation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for environment variable usage patterns
echo "Searching for environment variable usage patterns..."
rg -l "(UPLOADTHING_TOKEN|NUXT_UPLOADTHING_TOKEN)" --type md
# Check configuration documentation
echo "Checking configuration documentation..."
fd -e md -e mdx -x grep -l "UPLOADTHING_TOKEN" {} \;
Length of output: 1235
Script:
#!/bin/bash
# Let's check the specific documentation for Nuxt integration
echo "Checking Nuxt documentation for environment variable configuration..."
cat "./docs/src/app/(docs)/getting-started/nuxt/page.mdx"
# Let's also check the runtime config handling in the Nuxt package
echo "Checking runtime config handling..."
ast-grep --pattern 'const runtime = useRuntimeConfig()'
Length of output: 6205
examples/backend-adapters/server/package.json (2)
20-20: Verify the purpose and usage of @sinclair/typebox
A new runtime type system dependency (@sinclair/typebox) has been added. This addition should be verified against the PR objectives, particularly if it's related to the Nuxt module options fix.
#!/bin/bash
# Description: Check the usage of @sinclair/typebox in the codebase
# Search for TypeBox usage in the codebase
rg -g "*.{ts,js}" "@sinclair/typebox"
# Check if this is used in Nuxt-related files
rg -g "*nuxt*" "@sinclair/typebox"
# Look for any configuration schemas that might use TypeBox
ast-grep --pattern 'import { $_ } from "@sinclair/typebox"'15-16: Verify compatibility with the Effect ecosystem updates
The significant version jumps in the Effect ecosystem packages warrant careful verification:
- @effect/platform: 0.69.8 → 0.69.24
- @effect/platform-node: 0.64.9 → 0.64.26
- effect: 3.10.3 → 3.10.15
Also applies to: 23-23
✅ Verification successful
Effect ecosystem updates appear to be safe
The version updates are part of a coordinated dependency management across the codebase:
- The Effect packages are consistently used across multiple files, primarily for HTTP client/server functionality
- The version updates maintain compatibility between
@effect/platformand@effect/platform-node - Usage patterns in the codebase (particularly in
effect-platform.ts) suggest these are non-breaking updates
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for breaking changes in the Effect ecosystem updates
# Check the changelog/release notes for breaking changes
gh api graphql -f query='
{
repository(owner: "Effect-TS", name: "effect") {
releases(first: 10, orderBy: {field: CREATED_AT, direction: DESC}) {
nodes {
tagName
description
}
}
}
}'
# Verify that all Effect packages are in sync
rg -g "package.json" '"effect":' -A 1 -B 1
rg -g "package.json" '"@effect/platform":' -A 1 -B 1
rg -g "package.json" '"@effect/platform-node":' -A 1 -B 1
Length of output: 229
Script:
#!/bin/bash
# Let's try a different approach to verify Effect ecosystem dependencies
# Search for Effect-related dependencies across package.json files
fd package.json --exec grep -A 5 -B 5 '"@effect/'
# Check for any imports or usage of Effect packages
rg -g "*.{ts,js}" "@effect/platform"
rg -g "*.{ts,js}" "@effect/platform-node"
# Look for any migration guides or breaking changes documentation
fd "MIGRATION|CHANGELOG|BREAKING" -i
Length of output: 3253
packages/shared/package.json (1)
39-39: Verify Effect.ts version compatibility
The update to Effect.ts version 3.10.15 appears to be related to the Nuxt module options fix. Since this change is incorporated from Effect-TS repository PR #3833, let's verify the version compatibility and potential breaking changes.
✅ Verification successful
Let me gather more information about the usage of Effect.ts in the codebase to better understand the impact of this version update.
Based on the results, I can now generate a final response since we have sufficient information about the Effect.ts usage in the codebase.
Effect.ts version update appears safe to proceed
The update from 3.10.3 to 3.10.15 is a patch version increment that appears to be safe:
- Effect.ts is extensively used across the codebase, particularly in core functionality under
packages/sharedandpackages/uploadthing - The PR #3833 in Effect-TS repository is marked as a bug fix, not introducing breaking changes
- The version bump follows semantic versioning (patch version increment)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for breaking changes in effect package between versions
# Get the changelog or release notes for the relevant versions
gh api repos/Effect-TS/effect/releases | jq -r '.[] | select(.tag_name | contains("3.10.")) | {tag: .tag_name, body: .body}'
# Check for related changes in the Effect-TS PR
gh api repos/Effect-TS/effect/pulls/3833 | jq -r '.body'
Length of output: 1753
Script:
#!/bin/bash
# Check for Effect.ts imports and usage patterns
rg -l "from ['\"](effect|@effect)" --type ts
# Check package-lock.json for any potential dependency conflicts
fd package-lock.json --exec cat {} | jq '.packages | .[] | select(.name == "effect") | {version: .version, resolved: .resolved}'
# Look for Effect.ts specific configurations or settings
rg -l "effect" --type json --type ts
Length of output: 3968
packages/nuxt/src/module.ts (1)
49-55: Verify environment variable handling with these changes.
The explicit undefined values improve static analysis compatibility. However, given the PR objectives mentioning environment variable issues (UPLOADTHING_TOKEN vs NUXT_UPLOADTHING_TOKEN), we should verify the token handling.
Let's check how the token is handled across the codebase:
✅ Verification successful
Based on the shell script results, I can now generate the final response:
Environment variable handling is correctly implemented
The codebase consistently uses UPLOADTHING_TOKEN as the primary token variable, with special handling for Nuxt.js through the NUXT_ prefix. This is properly documented in the getting started guide:
docs/src/app/(docs)/getting-started/nuxt/page.mdxexplicitly states:NUXT_UPLOADTHING_TOKEN=... # 👆 NUXT_ prefix is **important**- The module correctly handles this by setting undefined defaults and using Nuxt's runtime config system:
nuxt.options.runtimeConfig.uploadthing
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify environment variable handling in the codebase
# Check for environment variable references
echo "Checking for environment variable references:"
rg -i "uploadthing_token|nuxt_uploadthing" -A 3
# Check for runtime config handling
echo "Checking runtime config handling:"
rg "runtimeConfig.uploadthing" -A 3
# Check for any documentation mentioning the token
echo "Checking documentation:"
fd -e md -e mdx | xargs rg -i "uploadthing.*token"
Length of output: 18049
packages/uploadthing/package.json (1)
153-153: LGTM! Verify latest stable versions.
The dependency version updates align with the PR objectives, particularly the incorporation of changes from the Effect-TS repository.
Let's verify these versions:
Also applies to: 156-156
✅ Verification successful
✅ Version updates are up-to-date and compatible
The dependency updates to @effect/[email protected] and [email protected] are using the latest patch versions available on npm, ensuring the package stays current while maintaining backward compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the latest stable versions and compatibility of effect packages
# Check latest versions on npm
echo "Checking @effect/platform versions:"
npm view @effect/platform versions --json | jq 'map(select(startswith("0.69."))) | .[-5:]'
echo "Checking effect versions:"
npm view effect versions --json | jq 'map(select(startswith("3.10."))) | .[-5:]'
# Check for breaking changes in the Effect-TS repository
echo "Checking for breaking changes in Effect-TS PR #3833:"
gh api repos/Effect-TS/effect/pulls/3833 --jq '.body' | grep -i "breaking"
Length of output: 641
| router: uploadRouter, | ||
| config: runtime.uploadthing, | ||
| config, | ||
| })(event); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for required configuration
The code should validate that required configuration properties are present before creating the route handler.
+ const requiredKeys = ['apiKey', 'secret'];
+ const missingKeys = requiredKeys.filter(key => !config[key]);
+ if (missingKeys.length > 0) {
+ throw new Error(`Missing required uploadthing configuration: ${missingKeys.join(', ')}`);
+ }
return createRouteHandler({
router: uploadRouter,
config,
})(event);📝 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.
| router: uploadRouter, | |
| config: runtime.uploadthing, | |
| config, | |
| })(event); | |
| const requiredKeys = ['apiKey', 'secret']; | |
| const missingKeys = requiredKeys.filter(key => !config[key]); | |
| if (missingKeys.length > 0) { | |
| throw new Error(`Missing required uploadthing configuration: ${missingKeys.join(', ')}`); | |
| } | |
| return createRouteHandler({ | |
| router: uploadRouter, | |
| config, | |
| })(event); |
| // Need to manually set these for static analysis to work, even if they're set to undefined | ||
| token: undefined, | ||
| isDev: undefined, | ||
| logLevel: undefined, |
There was a problem hiding this comment.
Should we set a default log level?
There was a problem hiding this comment.
it's set by core package
We just need to manually set all the options for the NUXT_ prefix to work
Closes #1047
Incorporates Effect-TS/effect#3833 + manual destruction of module options to work with static analysis
Summary by CodeRabbit
New Features
undefined.Bug Fixes
@uploadthing/nuxtmodule where specified options were not being honored.Dependency Updates
effectand@effect/platform.Documentation