-
Notifications
You must be signed in to change notification settings - Fork 381
fix: prevent internal access of deprecated getters #1224
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
base: main
Are you sure you want to change the base?
fix: prevent internal access of deprecated getters #1224
Conversation
🦋 Changeset detectedLatest commit: e8ba10f 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
|
@mcculloughrt-svh is attempting to deploy a commit to the Ping Labs Team on Vercel. A member of the Team first needs to authorize it. |
## Walkthrough
This change refactors how deprecated properties `url` and `appUrl` are handled during file uploads. It introduces internal getters to avoid repeated deprecation warnings and updates the SDK to emit a warning only when these deprecated properties are accessed, advising users to use alternative properties.
## Changes
| File(s) | Change Summary |
|-----------------------------------------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------|
| .changeset/tall-months-switch.md | Documents the patch addressing deprecation warnings for internal getter usage during uploads. |
| packages/uploadthing/src/_internal/upload-server.ts | Adds internal getter properties `_internalUrl` and `_internalAppUrl` to prevent triggering deprecation warnings internally. |
| packages/uploadthing/src/sdk/utils.ts | Implements deprecation warnings on `url` and `appUrl` getters in the upload result, advising migration to `ufsUrl`. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant SDK (uploadFile)
participant Server (uploadWithoutProgress)
User->>SDK (uploadFile): Call uploadFile()
SDK (uploadFile)->>Server (uploadWithoutProgress): Initiate upload
Server (uploadWithoutProgress)-->>SDK (uploadFile): Return response with _internalUrl, _internalAppUrl
SDK (uploadFile)-->>User: Return file object with deprecated getters (url, appUrl)
User->>SDK (uploadFile): Access file.url or file.appUrl
SDK (uploadFile)-->>User: Emit deprecation warning, return internal URL Possibly related PRs
Suggested reviewers
|
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.
Havne't had a chance to test it, but do we have the same issue server side?
Same issue with my self hosted version |
Not sure, I don't think so... I looked for everywhere else the deprecation warning was used, and traced from there forward and the only other location I came up with where it might be called inadvertently is in the handleCallbackRequest of _internal/handler.ts, or something that utilizes its result. I don't think this issue exists there but I'm not familiar enough with Effect to know what's happening inside some of that to be honest. |
The deprecated
url
andappUrl
getters from uploadWithoutProgress were being accessed internally by the uploadFile utility, triggering deprecation console warnings even if the end user is not accessing those getters. This keeps the deprecation warnings by shifting them onto the object returned by uploadFile, while accessing them internally through getters marked "_internal" that do not trigger the warning.See comment on issue from discord here
Summary by CodeRabbit
Bug Fixes
New Features
Style