-
Notifications
You must be signed in to change notification settings - Fork 334
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 effect deps #1095
chore: bump effect deps #1095
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
🦋 Changeset detectedLatest commit: fdb8a6e 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 |
WalkthroughThis pull request introduces updates to several package dependencies, specifically for Changes
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
commit: |
📦 Bundle size comparison
|
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] |
There was a problem hiding this 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
🧹 Outside diff range and nitpick comments (2)
packages/uploadthing/src/client.ts (2)
270-275
: Consider documenting the Effect upgrade impactThe error handling changes are part of a broader Effect library upgrade. Consider adding a note in the changelog about this breaking change in error handling patterns for consumers who might be directly handling these errors.
Test coverage needed for pause/resume and error handling
The verification process reveals that there are no dedicated test cases for the pause/resume functionality and the associated error handling (
UploadPausedError
andUploadAbortedError
). This is concerning given that:
- The code implements critical pause/resume functionality with error handling
- These features are exposed through public API methods (
pauseUpload
,resumeUpload
)- The error states are used across multiple framework integrations (React, Vue, Svelte, Solid)
Please add test cases to cover:
- Pausing and resuming uploads
- Error handling for interrupted uploads
- Abort scenarios
- Multiple file upload pause/resume states
🔗 Analysis chain
Line range hint
156-275
: Verify test coverage for error handling scenariosWhile the changes look correct, it's important to ensure that all error handling scenarios are covered in the test suite, particularly:
- Success cases with the new
Success
tag- Error cases with the new
Cause
tag- Interrupt handling for upload pausing
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Look for test files covering these scenarios echo "Checking for relevant test coverage..." fd -e test.ts -e spec.ts | xargs rg -l "uploadEffect|typedUploadFiles"Length of output: 159
Script:
#!/bin/bash # Let's try a broader search for test files and patterns echo "Looking for test files in the repository..." fd -e test.ts -e spec.ts echo -e "\nSearching for upload-related tests with broader patterns..." rg -l "upload.*test|test.*upload" --type ts echo -e "\nSearching for specific test cases..." rg -C 3 "pauseUpload|resumeUpload|UploadPausedError|UploadAbortedError" --type tsLength of output: 23141
Script:
#!/bin/bash # Let's check the test coverage for these specific error scenarios echo "Checking test coverage for error handling..." rg -C 5 "UploadPausedError|UploadAbortedError" packages/uploadthing/test/ echo -e "\nChecking test cases for pause/resume functionality..." rg -C 5 "pauseUpload|resumeUpload" packages/uploadthing/test/Length of output: 361
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.changeset/early-bananas-exist.md
(1 hunks)examples/backend-adapters/server/package.json
(1 hunks)packages/shared/package.json
(1 hunks)packages/uploadthing/package.json
(1 hunks)packages/uploadthing/src/client.ts
(3 hunks)playground-v6/package.json
(1 hunks)playground/package.json
(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- playground/package.json
- .changeset/early-bananas-exist.md
- playground-v6/package.json
- examples/backend-adapters/server/package.json
🔇 Additional comments (5)
packages/shared/package.json (1)
39-39
: Verify effect upgrade compatibility
The upgrade from [email protected]
to [email protected]
is a minor version bump. Let's verify the changes and potential impacts.
✅ Verification successful
Based on the results, I can generate the final response as we have sufficient information. The changelog shows that version 3.11.5 contains only patch changes focused on JSONSchema improvements, internal refactoring, and bug fixes. Additionally, the search results show that all packages in the monorepo using effect
are already aligned to version 3.11.5.
Effect upgrade is safe to proceed
The upgrade from [email protected]
to [email protected]
contains only non-breaking changes:
- JSONSchema improvements and bug fixes
- Internal refactoring changes
- All other packages in the monorepo are already using the same version
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the changelog between versions and any potential breaking changes
# Get the changelog for effect package between versions
gh api repos/Effect-TS/effect/releases | jq -r '.[] | select(.tag_name | contains("3.11.")) | .body'
# Check if there are any other packages in the monorepo using effect
rg '"effect":\s*"[^"]*"' --type json
Length of output: 10728
packages/uploadthing/package.json (2)
154-158
: Verify compatibility between effect packages
The PR updates both @effect/platform
(0.69.24 -> 0.70.7) and effect
(3.10.15 -> 3.11.5). Let's verify their compatibility and potential impacts.
✅ Verification successful
Based on the search results, I can verify the compatibility between the effect packages. The pnpm-lock.yaml shows that @effect/[email protected]
is correctly paired with [email protected]
as a peer dependency, and this combination is being used consistently across the project. Additionally, the codebase shows active usage of these packages in multiple files with proper imports.
The version update is safe and compatible
- The packages are correctly paired in the lockfile with
@effect/[email protected]
using[email protected]
as its peer dependency - The codebase shows proper usage of these packages in multiple files like
packages/uploadthing/src/sdk/utils.ts
,packages/uploadthing/src/internal/upload.server.ts
, etc. - The version bump follows semantic versioning (minor version updates)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check compatibility between effect packages and their changelogs
# Get the changelog for @effect/platform package
gh api repos/Effect-TS/platform/releases | jq -r '.[] | select(.tag_name | contains("0.70.")) | .body'
# Check if these versions are commonly used together in other projects
gh api graphql -f query='
query {
search(query: "\"@effect/platform\": \"0.70.7\" \"effect\": \"3.11.5\"", type: CODE, first: 5) {
nodes {
... on CodeResult {
repository {
nameWithOwner
}
file {
path
}
}
}
}
}'
Length of output: 1058
Script:
#!/bin/bash
# Let's try a different approach to check compatibility
# Check if there are any imports or usage of @effect/platform in the codebase
rg "@effect/platform" -A 2 -B 2
# Check for any effect-related imports to understand usage patterns
rg "from ['\"](effect|@effect)" -A 2 -B 2
# Look for any migration guides or documentation
fd -t f "README|CHANGELOG|MIGRATION" -X cat {}
Length of output: 79928
154-158
: Request context for the dependency updates
The dependency updates look consistent across packages:
@uploadthing/shared
: [email protected]uploadthing
: [email protected] and @effect/[email protected]
However, the PR description references a Discord issue without providing context about the specific problem being addressed. Could you please provide more details about the issue that these version bumps are fixing?
✅ Verification successful
Effect dependency updates appear to be safe and consistent
The dependency updates are well-aligned with the codebase usage:
- The
effect
package is extensively used throughout the codebase with numerous imports from various modules @effect/platform
is used in key server and client components for HTTP operations- Both packages are updated in lockstep (
[email protected]
and@effect/[email protected]
), maintaining version compatibility
The version bumps appear to be minor updates that shouldn't introduce breaking changes. No immediate issues or inconsistencies are found in the usage patterns that would be affected by these updates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any related changes in the codebase that might be affected by these updates
# Look for effect-related imports that might need updates
rg "from ['\"]effect/" --type typescript
# Look for @effect/platform imports
rg "from ['\"]@effect/platform" --type typescript
Length of output: 168
Script:
#!/bin/bash
# Let's try without the typescript file type flag since it's not recognized
# Look for effect-related imports
rg "from ['\"]effect/"
# Look for @effect/platform imports
rg "from ['\"]@effect/platform"
# Also check for direct requires
rg "require\(['\"]effect"
rg "require\(['\"]@effect/platform"
Length of output: 9427
packages/uploadthing/src/client.ts (2)
203-208
: LGTM! Consistent error handling pattern
The error handling changes in resumeUpload
follow the same pattern as the parent function, maintaining consistency in the Effect error handling approach.
156-161
: Verify consistent usage of new Effect error handling patterns
The change from Right
/Left
to Success
/Cause
aligns with the Effect library upgrade. Let's ensure this pattern is consistently applied across all Effect-based operations in the codebase.
✅ Verification successful
Effect error handling patterns have been consistently updated
The codebase shows consistent usage of the new Success
/Cause
pattern in packages/uploadthing/src/client.ts
. While Either
type definitions still exist in type files (packages/shared/src/types.ts
and packages/uploadthing/src/sdk/types.ts
), there are no remaining instances of the old Right
/Left
pattern in the actual implementation code. This confirms that the Effect library upgrade changes have been properly applied across all relevant code paths.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of the old Effect error handling pattern
# that might have been missed during the upgrade
echo "Checking for old Right/Left pattern usage..."
rg --type typescript '_tag === "(Right|Left)"'
echo "Checking for new Success/Cause pattern usage..."
rg --type typescript '_tag === "(Success|Cause)"'
Length of output: 374
Script:
#!/bin/bash
# Search for any remaining instances of the old Effect error handling pattern
# that might have been missed during the upgrade
echo "Checking for old Right/Left pattern usage..."
rg '_tag === "(Right|Left)"'
echo "Checking for new Success/Cause pattern usage..."
rg '_tag === "(Success|Cause)"'
# Also check for imports or uses of Either type which might indicate old pattern
echo "Checking for Either type usage..."
rg "Either[<,]"
Length of output: 785
fixes https://discord.com/channels/966627436387266600/1316504654225477662
Summary by CodeRabbit
Chores
"uploadthing"
and"@uploadthing/shared"
.Bug Fixes