Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: bump react peer dep and bunchee #1102

Closed
wants to merge 2 commits into from

Conversation

juliusmarminge
Copy link
Collaborator

@juliusmarminge juliusmarminge commented Dec 27, 2024

Summary by CodeRabbit

  • New Features

    • Introduced new schemas and classes for file uploads, including ContentDispositionSchema, ACLSchema, and various data classes for managing upload data.
  • Bug Fixes

    • Updated dependency versions to improve compatibility and performance, including bunchee and React versions.
  • Refactor

    • Restructured internal module paths from ./internal/ to ./_internal/ across multiple files for better organization.
  • Chores

    • Updated package configurations and peer dependencies to enhance compatibility with newer versions of libraries.
    • Added output path to the build task configuration for expanded build scope.

Copy link

vercel bot commented Dec 27, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs-uploadthing ❌ Failed (Inspect) Dec 30, 2024 10:34am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
legacy-docs-uploadthing ⬜️ Ignored (Inspect) Visit Preview Dec 30, 2024 10:34am

Copy link
Contributor

coderabbitai bot commented Dec 27, 2024

Walkthrough

This pull request involves a comprehensive update across multiple packages in the UploadThing project. The primary changes include updating the bunchee dependency version from ^5.6.2 to ^6.1.2 in several packages, modifying peer dependencies for broader version compatibility, and restructuring import paths from ./internal/ to ./_internal/. Additionally, a new TypeScript file for schemas related to file uploads has been introduced, while an older shared schemas file has been removed. These changes reflect a systematic update to the project's internal structure and dependency management.

Changes

Files Change Summary
packages/*/package.json Updated bunchee dependency from ^5.6.2 to ^6.1.2 across multiple packages
packages/expo/package.json Updated peer dependencies for expo-constants, expo-document-picker, expo-image-picker, and react to broader version compatibility
packages/uploadthing/package.json Removed ./internal/types export entry and updated peer dependencies, including removal of fastify and next
packages/uploadthing/src/_internal/config.ts Added /** #__PURE__ */ annotations to exported constants for potential tree-shaking optimization
packages/uploadthing/src/_internal/handler.ts Added /** #__PURE__ */ annotation to Context.Tag instantiation
Multiple packages/uploadthing/src/* files Replaced ./internal/ import paths with ./_internal/
packages/uploadthing/src/internal/shared-schemas.ts Removed file, likely replaced by new implementation
packages/uploadthing/src/_internal/shared-schemas.ts New file with schema definitions for file uploads and server interactions

Suggested labels

@uploadthing/react

Suggested reviewers

  • markflorkowski
  • t3dotgg

Possibly related PRs


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f1dd016 and 9683e64.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (7)
  • packages/expo/package.json (2 hunks)
  • packages/mime-types/package.json (1 hunks)
  • packages/react/package.json (2 hunks)
  • packages/shared/package.json (1 hunks)
  • packages/uploadthing/package.json (4 hunks)
  • packages/uploadthing/turbo.json (1 hunks)
  • packages/vue/package.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • packages/vue/package.json
  • packages/expo/package.json
  • packages/shared/package.json
  • packages/mime-types/package.json
  • packages/react/package.json
🔇 Additional comments (5)
packages/uploadthing/turbo.json (1)

7-7: LGTM: Build output configuration is properly updated

The addition of "dist/**" to the build outputs is consistent with the package structure changes and ensures Turborepo correctly tracks the dist directory artifacts.

Let's verify the build output configuration is consistent across the monorepo:

✅ Verification successful

Build output configuration is properly aligned across the monorepo

The verification shows that all packages in the monorepo have consistent build output configurations in their respective turbo.json files:

  • packages/uploadthing/turbo.json correctly includes all necessary output directories
  • packages/react/turbo.json includes "dist/**"
  • packages/shared/turbo.json includes "dist/**"
  • packages/mime-types/turbo.json includes "dist/**" among other directories
  • packages/expo/turbo.json includes "dist/**"
  • packages/nuxt/turbo.json includes "dist/**"

The addition of "dist/**" to the build outputs is consistent with the package structure changes and aligns with other packages in the monorepo.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for other turbo.json files that might need similar updates
fd -e json -x sh -c 'if grep -l "turbo" {} > /dev/null; then echo "=== {}" && cat {}; fi'

Length of output: 13065

packages/uploadthing/package.json (4)

Line range hint 119-132: LGTM: Files array properly updated

The files array has been correctly updated to:

  • Include the new dist directory
  • Remove the deprecated internal directory
  • Maintain all other necessary directories

138-138: LGTM: Clean script updated

The clean script has been properly updated to include .turbo in place of internal, which aligns with the project's restructuring.


159-159: Verify the bunchee upgrade impact

The bump from ^5.6.2 to ^6.1.2 is a major version upgrade that might include breaking changes.

Let's verify this upgrade across the monorepo:

✅ Verification successful

Let me fix the script and try again to get the necessary information.


Bunchee v6 upgrade appears safe to proceed

The verification shows:

  1. All packages in the monorepo are consistently using bunchee version ^6.1.2
  2. The breaking changes in v6 are minimal and only affect the CLI command syntax:
    • Changed bunchee --prepare to bunchee prepare

The upgrade appears safe as:

  • The breaking change only affects CLI usage, not the build output
  • The monorepo maintains version consistency across all packages
  • The new features (private shared modules, lint command) are optional additions
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for bunchee version consistency across packages
echo "=== Checking bunchee versions across packages"
fd package.json -x sh -c 'if grep -l "bunchee" {} > /dev/null; then echo "=== {}" && cat {} | jq -r ".devDependencies.bunchee // empty"' \;

# Check for any migration guides or breaking changes
echo "=== Checking for migration documentation"
gh api repos/huozhi/bunchee/releases | jq -r '.[].body' | grep -i "breaking"

Length of output: 2953


Script:

#!/bin/bash
# Check for bunchee version consistency across packages
echo "=== Checking bunchee versions across packages"
fd package.json | while read -r file; do
    if grep -q "bunchee" "$file"; then
        echo "=== $file"
        jq -r '.devDependencies.bunchee // empty' "$file"
    fi
done

# Check for any migration guides or breaking changes
echo -e "\n=== Checking for migration documentation"
gh api repos/huozhi/bunchee/releases | jq -r '.[] | select(.body | contains("Breaking")) | "Version: \(.tag_name)\n\(.body)\n"'

Length of output: 5302


176-179: Verify peer dependency changes

The peer dependencies have been cleaned up by:

  • Removing fastify and next
  • Updating tailwindcss from empty string to "*"

Let's verify these changes are safe:


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

pkg-pr-new bot commented Dec 27, 2024

Open in Stackblitz

More templates

@uploadthing/expo

npm i https://pkg.pr.new/pingdotgg/uploadthing/@uploadthing/expo@1102

@uploadthing/nuxt

npm i https://pkg.pr.new/pingdotgg/uploadthing/@uploadthing/nuxt@1102

@uploadthing/mime-types

npm i https://pkg.pr.new/pingdotgg/uploadthing/@uploadthing/mime-types@1102

@uploadthing/shared

npm i https://pkg.pr.new/pingdotgg/uploadthing/@uploadthing/shared@1102

@uploadthing/react

npm i https://pkg.pr.new/pingdotgg/uploadthing/@uploadthing/react@1102

uploadthing

npm i https://pkg.pr.new/pingdotgg/uploadthing@1102

@uploadthing/vue

npm i https://pkg.pr.new/pingdotgg/uploadthing/@uploadthing/vue@1102

commit: 9683e64

Copy link
Contributor

📦 Bundle size comparison

Bundle Size (gzip) Visualization
Main 29.30KB See Treemap 📊
PR (444341e) 29.30KB See Treemap 📊
Diff No change

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (4)
packages/uploadthing/src/_internal/config.ts (3)

22-22: Consider improving import.meta handling and environment configuration.

While the #__PURE__ annotation is correctly applied for tree-shaking optimization, the frustrated comment and type assertion for import.meta suggests this needs a more robust solution.

Consider:

  1. Using a proper type definition for import.meta.env
  2. Implementing a more structured fallback mechanism
  3. Adding runtime environment detection
-// fuck this I give up. import.meta is a mistake, someone else can fix it
-(import.meta as unknown as { env: Record<string, string> })?.env ?? {},
+// Handle both Vite's import.meta.env and regular process.env
+const getRuntimeEnv = () => {
+  if (typeof import.meta !== 'undefined' && import.meta.env) {
+    return import.meta.env as Record<string, string>;
+  }
+  return {};
+};
+getRuntimeEnv(),

62-65: LGTM! Comprehensive token validation with clear error messages.

The token configuration includes thorough validation and helpful error messages. The #__PURE__ annotation is correctly applied.

Consider adding troubleshooting steps to the error messages:

-"Missing token. Please set the `UPLOADTHING_TOKEN` environment variable or provide a token manually through config.",
+"Missing token. Please set the `UPLOADTHING_TOKEN` environment variable or provide a token manually through config. Visit docs.uploadthing.com/getting-started for setup instructions.",

85-85: Consider documenting multi-region support limitations.

The code correctly handles ingest URL configuration, but the comment "Currently only support 1 region per app" suggests a limitation that should be documented.

Consider:

  1. Adding a TODO comment with tracking issue for multi-region support
  2. Documenting this limitation in the package README
  3. Adding a warning log when multiple regions are provided but only the first is used
packages/uploadthing/src/_internal/shared-schemas.ts (1)

14-15: Ensure future extensibility for action types.

Defining ActionType using S.Literal("upload") is good for clarity. If more actions are introduced later, consider converting this literal definition into a broader union or array to accommodate easy expansions.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2248af0 and f1dd016.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (30)
  • packages/expo/package.json (2 hunks)
  • packages/mime-types/package.json (1 hunks)
  • packages/react/package.json (2 hunks)
  • packages/shared/package.json (1 hunks)
  • packages/uploadthing/package.json (3 hunks)
  • packages/uploadthing/src/_internal/config.ts (4 hunks)
  • packages/uploadthing/src/_internal/handler.ts (1 hunks)
  • packages/uploadthing/src/_internal/shared-schemas.ts (1 hunks)
  • packages/uploadthing/src/client.ts (1 hunks)
  • packages/uploadthing/src/effect-platform.ts (1 hunks)
  • packages/uploadthing/src/express.ts (1 hunks)
  • packages/uploadthing/src/fastify.ts (1 hunks)
  • packages/uploadthing/src/h3.ts (1 hunks)
  • packages/uploadthing/src/internal/shared-schemas.ts (0 hunks)
  • packages/uploadthing/src/next-legacy.ts (1 hunks)
  • packages/uploadthing/src/next.ts (1 hunks)
  • packages/uploadthing/src/remix.ts (1 hunks)
  • packages/uploadthing/src/sdk/index.ts (1 hunks)
  • packages/uploadthing/src/sdk/types.ts (1 hunks)
  • packages/uploadthing/src/sdk/utils.ts (1 hunks)
  • packages/uploadthing/src/server.ts (1 hunks)
  • packages/uploadthing/src/types.ts (2 hunks)
  • packages/uploadthing/test/__test-helpers.ts (1 hunks)
  • packages/uploadthing/test/adapters.test.ts (1 hunks)
  • packages/uploadthing/test/gen-uploader.test.ts (1 hunks)
  • packages/uploadthing/test/input.test.ts (1 hunks)
  • packages/uploadthing/test/request-handler.test.ts (1 hunks)
  • packages/uploadthing/test/sdk.test.ts (1 hunks)
  • packages/uploadthing/test/upload-builder.test.ts (1 hunks)
  • packages/vue/package.json (1 hunks)
💤 Files with no reviewable changes (1)
  • packages/uploadthing/src/internal/shared-schemas.ts
✅ Files skipped from review due to trivial changes (20)
  • packages/uploadthing/test/request-handler.test.ts
  • packages/shared/package.json
  • packages/uploadthing/test/sdk.test.ts
  • packages/uploadthing/src/sdk/types.ts
  • packages/uploadthing/src/h3.ts
  • packages/uploadthing/src/client.ts
  • packages/uploadthing/src/fastify.ts
  • packages/uploadthing/src/server.ts
  • packages/uploadthing/src/sdk/utils.ts
  • packages/uploadthing/src/types.ts
  • packages/uploadthing/test/adapters.test.ts
  • packages/uploadthing/src/_internal/handler.ts
  • packages/uploadthing/src/sdk/index.ts
  • packages/uploadthing/test/input.test.ts
  • packages/uploadthing/src/next.ts
  • packages/uploadthing/src/remix.ts
  • packages/uploadthing/src/effect-platform.ts
  • packages/uploadthing/test/upload-builder.test.ts
  • packages/uploadthing/src/express.ts
  • packages/uploadthing/src/next-legacy.ts
🔇 Additional comments (26)
packages/uploadthing/src/_internal/config.ts (2)

53-53: LGTM! Development mode configuration is well-structured.

The #__PURE__ annotation is correctly applied, and the fallback logic properly handles both Node.js and browser environments.


79-79: LGTM! API URL configuration handles validation properly.

The configuration includes proper URL validation and consistent trailing slash handling. The #__PURE__ annotation is correctly applied.

packages/uploadthing/package.json (3)

137-137: LGTM: Clean script update aligns with internal restructuring

The updated clean script now includes .turbo and removes internal, which aligns with the broader changes in the codebase structure.


177-177: LGTM: Improved tailwindcss peer dependency specification

Changed tailwindcss peer dependency from empty string to "*", making the version requirement more explicit while maintaining flexibility.


158-158: Verify bunchee upgrade impact

The upgrade of bunchee from ^5.6.2 to ^6.0.4 is a minor version bump. Let's verify there are no breaking changes.

✅ Verification successful

Breaking changes found in bunchee v6 upgrade

The upgrade from bunchee v5 to v6 includes breaking changes that need attention:

  • The --prepare flag has been changed to a prepare command
    • Old: bunchee --prepare
    • New: bunchee prepare

All packages in the monorepo are consistently using version ^6.0.4, so there's no version mismatch concern. However, if any of these packages use the --prepare flag in their build scripts, they need to be updated to use the new command syntax.

Let's verify if any package uses the old flag syntax:


Let me try a different approach to check the build scripts:


Bunchee v6 upgrade is safe to proceed

The breaking change in bunchee v6 involves changing bunchee --prepare to bunchee prepare, but after checking all package.json scripts in the monorepo:

  • None of the packages use the --prepare flag
  • All packages are consistently using version ^6.0.4
  • Current usage patterns (--tsconfig, -w, --no-clean) are not affected by the breaking changes

The upgrade should not cause any issues in the build process.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for breaking changes in bunchee upgrade

# Get the changelog or release notes for bunchee
gh api repos/huozhi/bunchee/releases | jq -r '.[].body' | grep -A 10 "6.0.0"

# Check if any other packages in the monorepo are using a different version
fd package.json | xargs grep -l "bunchee" | xargs cat | jq 'select(.dependencies.bunchee or .devDependencies.bunchee) | {name: .name, version: (.dependencies.bunchee // .devDependencies.bunchee)}'

Length of output: 1865


Script:

#!/bin/bash
# Check for usage of bunchee --prepare in package.json scripts
fd package.json | xargs cat | jq -r '.scripts | to_entries | .[] | select(.value | contains("bunchee --prepare")) | "\(.key): \(.value)"'

Length of output: 298


Script:

#!/bin/bash
# Check for any usage of bunchee in package.json scripts
fd package.json | xargs cat | jq -r '.scripts | select(. != null) | to_entries[] | select(.value | contains("bunchee")) | "\(.key): \(.value)"'

Length of output: 1020

packages/expo/package.json (1)

58-58: LGTM: React peer dependency update

Adding support for React 19 while maintaining compatibility with React 17 and 18 is a good approach.

packages/vue/package.json (1)

50-50: LGTM: Consistent bunchee version update

The bunchee update to ^6.0.4 aligns with the changes across other packages.

packages/mime-types/package.json (1)

92-92: LGTM: Consistent bunchee version update

The bunchee update to ^6.0.4 aligns with the changes across other packages.

packages/react/package.json (2)

85-85: Verify bunchee major version bump impact

The bump from bunchee 5.x.x to 6.0.4 is a major version change. Please ensure:

  • No breaking changes affect the build output
  • Build scripts remain compatible
✅ Verification successful

Bunchee version 6.0.4 is consistently used across packages

The verification shows:

  • All packages using bunchee have been updated to version ^6.0.4
  • Build scripts across packages use standard bunchee commands with consistent flags (--tsconfig, -w, --no-clean)
  • No custom bunchee configuration files exist that would need updates

Since all packages are using the same major version and the build scripts follow a consistent pattern without complex customization, the version bump is properly synchronized across the monorepo. The standard usage of bunchee commands suggests minimal risk from the major version update.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if bunchee version is consistent across all packages
fd -e json -x jq -r 'select(.devDependencies.bunchee != null) | "\(.name): \(.devDependencies.bunchee)"' {} \;

# Look for any bunchee config files that might need updates
fd -g '*bunchee*'

Length of output: 868


Script:

#!/bin/bash
# Check git history for bunchee version changes and related commits
git log -p --all --grep="bunchee" -- '**/package.json'

# Look for build scripts that might use bunchee
rg -l "bunchee" -g "package.json" | xargs jq -r '.scripts | to_entries | .[] | select(.value | contains("bunchee")) | "\(.key): \(.value)"'

Length of output: 1012


70-70: Verify React 19 compatibility strategy

Adding React 19 to peer dependencies is forward-looking but requires careful consideration:

  • Ensure the package's features are tested against React 19 alpha/beta
  • Consider documenting any React 19 specific behaviors or limitations
  • Verify if this aligns with the project's compatibility strategy
packages/uploadthing/test/gen-uploader.test.ts (1)

4-4: Good consistent internal path update.

This import path change aligns with the broader _internal folder reorganization. The rest of the test file remains unchanged, so no further action is required here.

packages/uploadthing/src/_internal/shared-schemas.ts (15)

1-4: Imports are well-structured and minimal.

The introduced imports from effect/Schema and @uploadthing/shared are necessary for schema definitions. This organization appears clean and maintainable.


6-9: Schemas for content disposition and ACL look correct.

Using S.Literal with spread operators helps ensure valid enumerations for content dispositions and ACL. This approach is concise and robust.


20-21: Hook literal definition is clear.

Defining "callback" and "error" as valid hooks is straightforward. This ensures strict type checking and is safe to extend as new hooks are introduced.


27-34: String decoding logic is well-implemented.

Using S.transform with TextEncoder/TextDecoder is a clean approach for handling string encoding and decoding. This ensures minimal room for encoding errors.


36-44: Secure pattern for API key redaction.

Using S.Redacted for the API key is a best practice. The default ingest host is also sensible. This approach neatly addresses security concerns.


45-48: Composing transformations is exemplary.

Chaining Uint8ArrayFromBase64 -> DecodeString -> parseJson(ParsedToken) yields a succinct flow for decoding the token. This clarifies how input data is sequentially processed.


59-65: Structured approach to file metadata.

Defining the FileUploadData class with typed fields makes the file data schema explicit, which will catch potential mismatches early.


70-75: Extension pattern is consistent.

Using .extend for FileUploadDataWithCustomId keeps the parent schema in sync, improving reusability of base file properties.


84-92: Easily managed additional file properties.

This extension to handle url, appUrl, and fileHash is logically separated from base file data. The approach is consistent and adheres to open/closed principles.


97-103: Sufficient interface for client data.

ClientUploadedFileData<T> wraps UploadedFileData and includes a typed serverData. This improves the developer experience by strongly typing the callback’s return value.


111-118: NewPresignedUrl is clearly defined.

It’s good that customId can be null; this ensures optional fields are handled explicitly in typed form.


119-127: Structured stream part payload.

Having a dedicated schema (MetadataFetchStreamPart) with hook validated against a known literal is a reliable way to enforce valid inbound events.


128-134: Minimal success property conveys result clearly.

MetadataFetchResponse with an ok boolean property is an elegant pattern for wide usage across multiple layers, avoiding extraneous fields.


135-140: Consistent success response pattern.

CallbackResultResponse is similarly straightforward and aligns with the other response classes. Maintain a consistent approach across response schemas.


148-152: Upload action payload is well-defined.

UploadActionPayload references reusable schemas for files and input. This keeps logic DRY and consistent with existing definitions.

Comment on lines +55 to +57
"expo-constants": "*",
"expo-document-picker": "*",
"expo-image-picker": "*",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Keep the current version constraints for expo dependencies

Based on the codebase analysis, I recommend keeping the current version constraints instead of using "*" for the following reasons:

  • These expo packages are actively used in the codebase:
    • expo-constants is imported in packages/expo/src/index.ts and examples/minimal-expo/lib/trpc.tsx
    • expo-document-picker is used in packages/expo/src/document-picker.ts
    • expo-image-picker is used in packages/expo/src/image-picker.ts

The current version constraints in devDependencies are:

  • expo-constants: ^15.4.5
  • expo-document-picker: ^12.0.1
  • expo-image-picker: ^15.0.4

These provide a good balance between stability (minimum version) and flexibility (caret range for updates). Using "*" could lead to unexpected breaking changes as these packages have major version jumps (current latest versions are 17.0.3, 13.0.1, and 16.0.3 respectively).

🔗 Analysis chain

Consider maintaining version constraints for expo dependencies

Relaxing all expo peer dependencies to "*" removes version constraints which could lead to compatibility issues. Consider maintaining minimum version bounds while allowing flexibility for newer versions.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check latest versions and changelogs for expo packages
for pkg in expo-constants expo-document-picker expo-image-picker; do
  echo "Checking $pkg..."
  curl -s "https://registry.npmjs.org/$pkg" | jq -r '.["dist-tags"].latest'
done

Length of output: 689


Script:

#!/bin/bash
# Check for actual usage of these expo packages in the codebase
echo "Searching for expo-constants usage..."
rg "expo-constants" -B2 -A2

echo -e "\nSearching for expo-document-picker usage..."
rg "expo-document-picker" -B2 -A2

echo -e "\nSearching for expo-image-picker usage..."
rg "expo-image-picker" -B2 -A2

Length of output: 13672

Comment on lines +9 to +12
import { UPLOADTHING_VERSION } from "../src/_internal/config";
import { ParsedToken, UploadThingToken } from "../src/_internal/shared-schemas";
import type { ActionType } from "../src/_internal/shared-schemas";
import type { UploadPutResult } from "../src/_internal/types";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Documentation links need updating to reflect new _internal structure

The imports in the test file correctly use the new _internal structure. However, the documentation contains outdated GitHub links pointing to the old internal path. The files exist at:

  • packages/uploadthing/src/_internal/handler.ts
  • packages/uploadthing/src/_internal/types.ts
  • docs/src/app/(docs)/uploading-files/page.mdx: Update the GitHub links to point to the new _internal paths for both handler.ts and types.ts references.
🔗 Analysis chain

Path updates match _internal restructure.

These updated imports reflect the new _internal structure without apparent regressions. Ensure that these exports remain stable, and confirm that no references in the codebase remain to the old internal path.


Let me verify if these references in the documentation are outdated by checking their current state.


Let me check if these files exist in the new _internal structure and verify the documentation links are outdated.


Let me check the actual paths in the packages directory to verify the structure.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Validate that no references to ../src/internal/ remain
rg '../src/internal/'

Length of output: 435


Script:

#!/bin/bash
# Check if these files still exist at the old paths
fd "handler.ts|types.ts" src/internal/

# Check the current content of these documentation files
rg -A 2 "uploadthing/src/internal" docs/

Length of output: 975


Script:

#!/bin/bash
# Check if these files exist in the new _internal structure
fd "handler.ts|types.ts" src/_internal/

# Check the actual content of these documentation files to see if they need updating
cat docs/src/app/\(docs\)/uploading-files/page.mdx

Length of output: 21437


Script:

#!/bin/bash
# Check the actual structure in packages/uploadthing
fd . packages/uploadthing/src/

# Check if the referenced files exist in the new _internal structure
fd "handler.ts|types.ts" packages/uploadthing/src/_internal/

Length of output: 1792

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/uploadthing/src/_internal/shared-schemas.ts (1)

97-103: Interface design is flexible yet typed.
Using a generic type parameter for serverData is a solid approach to support different server-side callback responses.

Consider also providing a default type parameter for serverData to simplify usage when no custom type is needed.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2248af0 and f1dd016.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (30)
  • packages/expo/package.json (2 hunks)
  • packages/mime-types/package.json (1 hunks)
  • packages/react/package.json (2 hunks)
  • packages/shared/package.json (1 hunks)
  • packages/uploadthing/package.json (3 hunks)
  • packages/uploadthing/src/_internal/config.ts (4 hunks)
  • packages/uploadthing/src/_internal/handler.ts (1 hunks)
  • packages/uploadthing/src/_internal/shared-schemas.ts (1 hunks)
  • packages/uploadthing/src/client.ts (1 hunks)
  • packages/uploadthing/src/effect-platform.ts (1 hunks)
  • packages/uploadthing/src/express.ts (1 hunks)
  • packages/uploadthing/src/fastify.ts (1 hunks)
  • packages/uploadthing/src/h3.ts (1 hunks)
  • packages/uploadthing/src/internal/shared-schemas.ts (0 hunks)
  • packages/uploadthing/src/next-legacy.ts (1 hunks)
  • packages/uploadthing/src/next.ts (1 hunks)
  • packages/uploadthing/src/remix.ts (1 hunks)
  • packages/uploadthing/src/sdk/index.ts (1 hunks)
  • packages/uploadthing/src/sdk/types.ts (1 hunks)
  • packages/uploadthing/src/sdk/utils.ts (1 hunks)
  • packages/uploadthing/src/server.ts (1 hunks)
  • packages/uploadthing/src/types.ts (2 hunks)
  • packages/uploadthing/test/__test-helpers.ts (1 hunks)
  • packages/uploadthing/test/adapters.test.ts (1 hunks)
  • packages/uploadthing/test/gen-uploader.test.ts (1 hunks)
  • packages/uploadthing/test/input.test.ts (1 hunks)
  • packages/uploadthing/test/request-handler.test.ts (1 hunks)
  • packages/uploadthing/test/sdk.test.ts (1 hunks)
  • packages/uploadthing/test/upload-builder.test.ts (1 hunks)
  • packages/vue/package.json (1 hunks)
💤 Files with no reviewable changes (1)
  • packages/uploadthing/src/internal/shared-schemas.ts
✅ Files skipped from review due to trivial changes (20)
  • packages/uploadthing/src/sdk/types.ts
  • packages/uploadthing/test/input.test.ts
  • packages/uploadthing/src/remix.ts
  • packages/uploadthing/src/server.ts
  • packages/uploadthing/src/next-legacy.ts
  • packages/uploadthing/src/effect-platform.ts
  • packages/uploadthing/src/h3.ts
  • packages/uploadthing/src/sdk/utils.ts
  • packages/uploadthing/test/sdk.test.ts
  • packages/uploadthing/test/request-handler.test.ts
  • packages/uploadthing/src/sdk/index.ts
  • packages/uploadthing/src/next.ts
  • packages/uploadthing/test/adapters.test.ts
  • packages/uploadthing/test/upload-builder.test.ts
  • packages/uploadthing/src/_internal/handler.ts
  • packages/uploadthing/src/fastify.ts
  • packages/uploadthing/src/express.ts
  • packages/uploadthing/src/_internal/config.ts
  • packages/uploadthing/src/types.ts
  • packages/uploadthing/test/__test-helpers.ts
🔇 Additional comments (17)
packages/uploadthing/src/client.ts (1)

17-20: LGTM! Import path changes align with the broader refactoring.

The changes to import paths from ./internal/ to ./_internal/ are consistent with the project-wide restructuring mentioned in the PR summary.

Let's verify that all imports from the internal directory have been consistently updated across the codebase:

✅ Verification successful

All imports consistently use the new ./_internal/ path structure

The verification results show that all TypeScript imports across the codebase are using the new ./_internal/ path structure. There are no remaining imports using the old ./internal/ path, confirming that the migration was complete and consistent.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify consistent usage of the new _internal directory structure
# Look for any remaining imports from the old ./internal/ path
rg "from ['\"].*?/internal/" --type ts

# Look for imports from the new ./_internal/ path to confirm the migration
rg "from ['\"].*?/_internal/" --type ts

Length of output: 6431

packages/uploadthing/test/gen-uploader.test.ts (1)

4-4: Confirm consistency with the _internal directory structure.

The import path has been updated to reference _internal. Ensure that other test files and imports in the codebase have been updated accordingly to maintain consistency and avoid missing import paths or unresolved modules.

Run this script to confirm references to _internal are consistent throughout the codebase:

✅ Verification successful

Let me try a different approach to verify the consistency of internal directory references.


All imports consistently use the _internal path structure

Based on the search results, all imports in the test files are consistently using the new _internal path structure. The only references to the old internal path (without underscore) are in documentation files pointing to historical GitHub links, which is not a concern as they are just reference links to specific commits.

The verification shows that:

  • All test files import from ../src/_internal/
  • No active code references the old src/internal path
  • The migration to _internal appears to be complete and consistent
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for references to the old "internal" path in the entire repository.
rg "\.\.\/src/internal"

Length of output: 114


Script:

#!/bin/bash
# Search for references to both "internal" and "_internal" paths in the codebase
rg --no-heading "from ['\"].*src/[_]?internal"

Length of output: 1465


Script:

#!/bin/bash
# Find all files in the internal or _internal directory
fd -t f "." -p "src/[_]?internal"

Length of output: 137


Script:

#!/bin/bash
# Search for import statements containing 'internal' with proper escaping
rg --no-heading "from '\.\./src/[_]?internal"

Length of output: 50


Script:

#!/bin/bash
# Let's find all files in the src directory structure to understand the internal directory
fd -t f . "src"

Length of output: 106


Script:

#!/bin/bash
# Search for any remaining references to plain "internal" (without underscore)
rg --no-heading "src/internal"

Length of output: 444

packages/uploadthing/package.json (3)

137-137: LGTM: Clean script update aligns with directory structure changes

The clean script has been updated to include .turbo and remove internal, which aligns with the broader changes to move away from the internal directory structure.


177-177: LGTM: Explicit tailwindcss peer dependency

Changed tailwindcss peer dependency from empty string to "*", making the version requirement more explicit while maintaining flexibility.


158-158: Verify the impact of bunchee major version upgrade

The upgrade from bunchee ^5.6.2 to ^6.0.4 is a major version bump that might include breaking changes.

Let's verify the changelog and any potential breaking changes:

packages/uploadthing/src/_internal/shared-schemas.ts (5)

1-5: Imports look good and consistent.
No issues detected. They follow conventional best practices for TypeScript import syntax.


6-9: Clarity of content disposition and ACL schemas.
The usage of S.Literal(...ValidContentDispositions) and S.Literal(...ValidACLs) helps constrain valid values at compile- and runtime.


27-34: Potential browser compatibility issue with TextEncoder/TextDecoder.
While these APIs are widely supported in modern browsers and Node.js, consider verifying if your environment or build system transpiles these for legacy browsers.


36-43: Security reminder regarding S.Redacted.
Be mindful that S.Redacted ensures sensitive data is never serialized as plain text for logs or error messages. Confirm that downstream logic does not unintentionally reveal apiKey.


148-152: Schema ensures consistent payload structure for uploads.
This model clearly enforces the shape of the request payload.

packages/shared/package.json (1)

46-46: LGTM: Bunchee version bump

The update of bunchee to ^6.0.4 aligns with the PR objectives.

packages/expo/package.json (2)

43-43: LGTM: Version bumps for bunchee and React

The updates align with PR objectives:

  • Bunchee updated to ^6.0.4
  • React peer dependency expanded to support v19

Also applies to: 58-58


55-57: 🛠️ Refactor suggestion

Consider keeping version constraints for Expo dependencies

Changing Expo dependencies to wildcard "*" removes upper bounds that protected against breaking changes. This could lead to unexpected compatibility issues when new major versions are released.

Consider keeping semver ranges to ensure compatibility:

-    "expo-constants": "*",
-    "expo-document-picker": "*",
-    "expo-image-picker": "*",
+    "expo-constants": ">=15.4.5",
+    "expo-document-picker": ">=11.0.0",
+    "expo-image-picker": ">=14.0.0"

Let's check the latest major versions and any breaking changes:

✅ Verification successful

Let me gather more context about the Expo SDK version used in the project to recommend appropriate version constraints.


Let me check the main expo package version from the root package.json to ensure we recommend compatible versions.


Let me try one more approach to find the Expo version by checking for yarn.lock or any other package manager lock files.


Let me check the example Expo project to understand the recommended Expo version.


Keep version constraints aligned with Expo SDK 51

Based on the minimal example using Expo SDK 51.0.8 and the devDependencies in the package, I recommend using these version constraints to ensure compatibility:

-    "expo-constants": "*",
-    "expo-document-picker": "*",
-    "expo-image-picker": "*",
+    "expo-constants": ">=15.4.5 <18.0.0",
+    "expo-document-picker": ">=12.0.1 <14.0.0",
+    "expo-image-picker": ">=15.0.4 <17.0.0"

These ranges:

  • Allow patches and minor updates for SDK 51 compatibility
  • Prevent major version jumps that could break compatibility
  • Are based on the devDependencies versions and latest available versions
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check latest versions of expo packages
for pkg in expo-constants expo-document-picker expo-image-picker; do
  echo "Checking $pkg..."
  curl -s "https://registry.npmjs.org/$pkg" | jq -r '.["dist-tags"].latest'
done

Length of output: 689


Script:

#!/bin/bash
# Check expo SDK version from package.json
cat packages/expo/package.json | jq -r '.dependencies.expo'

# Also check if there are any peer dependencies specified
cat packages/expo/package.json | jq -r '.peerDependencies // empty'

Length of output: 315


Script:

#!/bin/bash
# Check expo version from root package.json
cat package.json | jq -r '.dependencies.expo // empty'

# Also check workspace dependencies
cat package-lock.json | jq -r '.packages."node_modules/expo".version // empty'

Length of output: 187


Script:

#!/bin/bash
# Check if yarn.lock exists and try to find expo version
if [ -f "yarn.lock" ]; then
  echo "Found yarn.lock"
  grep -A 2 '^"expo@' yarn.lock
fi

# List all package.json files to see project structure
fd package.json

Length of output: 1517


Script:

#!/bin/bash
# Check expo version in the minimal example
cat examples/minimal-expo/package.json | jq -r '.dependencies.expo'

# Also check package.json content in expo package
cat packages/expo/package.json | jq -r '.'

Length of output: 2146

packages/vue/package.json (1)

50-50: LGTM: Bunchee version bump

The update of bunchee to ^6.0.4 aligns with the PR objectives.

packages/mime-types/package.json (1)

92-92: Verify breaking changes in bunchee v6

The update from bunchee v5 to v6 is a major version bump. Please ensure there are no breaking changes that could affect the build process.

Run the following script to check the changelog:

✅ Verification successful

Breaking change in bunchee v6 is minor and only affects CLI usage

Based on the changelog, the breaking change in bunchee v6 is limited to renaming the CLI flag --prepare to a subcommand prepare. Since this is only used in build scripts, we should verify if the package is using this flag.


Bunchee v6 upgrade is safe to proceed

The breaking change in bunchee v6 only affects the --prepare CLI flag, which is not used in this package. The current build scripts use --tsconfig and -w flags that remain unchanged. Additionally, v6 brings improvements like better tree-shaking and Windows compatibility.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for breaking changes in bunchee v6

# Fetch the package's releases
gh api graphql -f query='
{
  repository(owner: "huozhi", name: "bunchee") {
    releases(first: 10, orderBy: {field: CREATED_AT, direction: DESC}) {
      nodes {
        tagName
        description
      }
    }
  }
}' | jq -r '.data.repository.releases.nodes[] | select(.tagName | startswith("v6.")) | .description'

Length of output: 2519


Script:

#!/bin/bash
# Check if the prepare flag is used in package.json scripts
rg -l "bunchee.*--prepare" packages/mime-types/package.json

# Also check any build or configuration files
fd -e js -e json -e sh "build|config" packages/mime-types/ -x rg -l "bunchee.*--prepare" {}

Length of output: 157


Script:

#!/bin/bash
# Let's check how bunchee is actually used in the package
cat packages/mime-types/package.json | jq -r '.scripts | to_entries | .[] | select(.value | contains("bunchee"))'

# Also check if there are any build configuration files
fd . packages/mime-types/ -e js -e json -e ts -e cjs -e mjs --exclude node_modules

Length of output: 733

packages/react/package.json (2)

70-70: LGTM! Forward-compatible React peer dependency

The change proactively adds support for React 19 while maintaining compatibility with existing React 17 and 18 versions. This is a good practice for smooth future upgrades.


85-85: Consistent bunchee version bump

This change aligns with the same version bump in other packages.

Copy link

changeset-bot bot commented Dec 30, 2024

⚠️ No Changeset found

Latest commit: 9683e64

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant