fix: url creation #1240
fix: url creation #1240
Conversation
🦋 Changeset detectedLatest commit: 3f65cee 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 |
|
@HamzaNa1 is attempting to deploy a commit to the Ping Labs Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughReplaces use of EXPO_PUBLIC_SERVER_ORIGIN with EXPO_PUBLIC_SERVER_URL and updates URL resolution to prefer EXPO_PUBLIC_SERVER_URL, then window.location.origin (when available), then http://{debuggerHost}. Adjusts warning message accordingly. No exported/public API changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant ExpoPkg as uploadthing/expo
participant Env as process.env
participant Window as window.location
participant Debugger as debuggerHost
App->>ExpoPkg: generateReactNativeHelpers(initOpts)
Note over ExpoPkg: Build base URL
ExpoPkg->>Env: Read EXPO_PUBLIC_SERVER_URL
alt EXPO_PUBLIC_SERVER_URL set
ExpoPkg->>ExpoPkg: Use env URL (absolute or resolve relative)
else not set
ExpoPkg->>Window: Check window.location.origin
alt window available
ExpoPkg->>ExpoPkg: Use window.location.origin
else no window
ExpoPkg->>Debugger: Use http://{debuggerHost}
end
end
ExpoPkg-->>App: Resolved URL (or warn on failure)
Note right of ExpoPkg: Warning references EXPO_PUBLIC_SERVER_URL
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/expo/src/index.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
| process.env.EXPO_PUBLIC_SERVER_URL ?? | ||
| (typeof window !== "undefined" && window.location?.origin | ||
| ? window.location?.origin | ||
| : `http://${debuggerHost}`), | ||
| ); | ||
| } catch (err) { | ||
| // Can't throw since window.location is undefined in Metro pass | ||
| // but may get defined when app mounts. | ||
| // eslint-disable-next-line no-console | ||
| console.warn( | ||
| `Failed to resolve URL from ${initOpts?.url?.toString()} and ${process.env.EXPO_PUBLIC_SERVER_ORIGIN} or ${debuggerHost}. Your application may not work as expected.`, | ||
| `Failed to resolve URL from ${initOpts?.url?.toString()} and ${process.env.EXPO_PUBLIC_SERVER_URL} or ${debuggerHost}. Your application may not work as expected.`, | ||
| err, |
There was a problem hiding this comment.
Restore support for EXPO_PUBLIC_SERVER_ORIGIN.
This change drops the old env key entirely, so any existing app that already had EXPO_PUBLIC_SERVER_ORIGIN configured (which was required up to now) will silently revert to the window/debuggerHost fallback. In React Native contexts without window (the bug we’re fixing), that means we now end up on http://${debuggerHost}—i.e. broken uploads after upgrading. Please keep the old variable as a fallback (and reflect it in the warning) so existing deployments don’t regress.
- url = new URL(
- initOpts?.url ?? "/api/uploadthing",
- process.env.EXPO_PUBLIC_SERVER_URL ??
- (typeof window !== "undefined" && window.location?.origin
- ? window.location?.origin
- : `http://${debuggerHost}`),
- );
+ url = new URL(
+ initOpts?.url ?? "/api/uploadthing",
+ process.env.EXPO_PUBLIC_SERVER_URL ??
+ process.env.EXPO_PUBLIC_SERVER_ORIGIN ??
+ (typeof window !== "undefined" && window.location?.origin
+ ? window.location?.origin
+ : `http://${debuggerHost}`),
+ );
@@
- `Failed to resolve URL from ${initOpts?.url?.toString()} and ${process.env.EXPO_PUBLIC_SERVER_URL} or ${debuggerHost}. Your application may not work as expected.`,
+ `Failed to resolve URL from ${initOpts?.url?.toString()} and ${
+ process.env.EXPO_PUBLIC_SERVER_URL ?? process.env.EXPO_PUBLIC_SERVER_ORIGIN
+ } or ${debuggerHost}. Your application may not work as expected.`,📝 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.
| process.env.EXPO_PUBLIC_SERVER_URL ?? | |
| (typeof window !== "undefined" && window.location?.origin | |
| ? window.location?.origin | |
| : `http://${debuggerHost}`), | |
| ); | |
| } catch (err) { | |
| // Can't throw since window.location is undefined in Metro pass | |
| // but may get defined when app mounts. | |
| // eslint-disable-next-line no-console | |
| console.warn( | |
| `Failed to resolve URL from ${initOpts?.url?.toString()} and ${process.env.EXPO_PUBLIC_SERVER_ORIGIN} or ${debuggerHost}. Your application may not work as expected.`, | |
| `Failed to resolve URL from ${initOpts?.url?.toString()} and ${process.env.EXPO_PUBLIC_SERVER_URL} or ${debuggerHost}. Your application may not work as expected.`, | |
| err, | |
| url = new URL( | |
| initOpts?.url ?? "/api/uploadthing", | |
| process.env.EXPO_PUBLIC_SERVER_URL ?? | |
| process.env.EXPO_PUBLIC_SERVER_ORIGIN ?? | |
| (typeof window !== "undefined" && window.location?.origin | |
| ? window.location?.origin | |
| : `http://${debuggerHost}`), | |
| ); | |
| } catch (err) { | |
| // Can't throw since window.location is undefined in Metro pass | |
| // but may get defined when app mounts. | |
| // eslint-disable-next-line no-console | |
| console.warn( | |
| `Failed to resolve URL from ${initOpts?.url?.toString()} and ${ | |
| process.env.EXPO_PUBLIC_SERVER_URL ?? process.env.EXPO_PUBLIC_SERVER_ORIGIN | |
| } or ${debuggerHost}. Your application may not work as expected.`, | |
| err, |
🤖 Prompt for AI Agents
In packages/expo/src/index.ts around lines 59 to 70, the code removed fallback
support for the legacy EXPO_PUBLIC_SERVER_ORIGIN env var causing RN contexts
without window to fall back to http://${debuggerHost}; restore support by
checking process.env.EXPO_PUBLIC_SERVER_URL first, then
process.env.EXPO_PUBLIC_SERVER_ORIGIN, then the window.location origin fallback
and finally http://${debuggerHost}; update the warning logged in the catch to
mention EXPO_PUBLIC_SERVER_ORIGIN alongside EXPO_PUBLIC_SERVER_URL and
debuggerHost so existing deployments are not regressed and users see the correct
env variables referenced.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/tough-files-leave.md (1)
5-5: Tighten up the sentence for readability.The run-on sentence makes the changeset harder to scan. Please break it into shorter sentences or add proper punctuation so the timeline of fallbacks is clearer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/tough-files-leave.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Vercel Agent Review
|
Update examples and turbo.json. Keep old (incorrect) variable as a potential fallback to prevent issues for anyone who worked around this previously |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| * If relative, host will be inferred from either the `EXPO_PUBLIC_SERVER_URL` environment variable or `ExpoConstants.hostUri` | ||
| * | ||
| * @default (process.env.EXPO_PUBLIC_SERVER_ORIGIN ?? ExpoConstants.debuggerHost) + "/api/uploadthing" | ||
| * @default (process.env.EXPO_PUBLIC_SERVER_URL ?? ExpoConstants.debuggerHost) + "/api/uploadthing" |
There was a problem hiding this comment.
| * If relative, host will be inferred from either the `EXPO_PUBLIC_SERVER_URL` environment variable or `ExpoConstants.hostUri` | |
| * | |
| * @default (process.env.EXPO_PUBLIC_SERVER_ORIGIN ?? ExpoConstants.debuggerHost) + "/api/uploadthing" | |
| * @default (process.env.EXPO_PUBLIC_SERVER_URL ?? ExpoConstants.debuggerHost) + "/api/uploadthing" | |
| * If relative, host will be inferred from either the `EXPO_PUBLIC_SERVER_URL` environment variable or `Constants.expoConfig?.hostUri` | |
| * | |
| * @default (process.env.EXPO_PUBLIC_SERVER_URL ?? Constants.expoConfig?.hostUri) + "/api/uploadthing" |
JSDoc comments reference ExpoConstants.debuggerHost but the actual code uses Constants.expoConfig?.hostUri, creating documentation inconsistency.
View Details
Analysis
JSDoc comments reference incorrect ExpoConstants properties
What fails: JSDoc documentation in packages/expo/src/index.ts references non-existent ExpoConstants.debuggerHost and ExpoConstants.hostUri properties, while the actual implementation correctly uses Constants.expoConfig?.hostUri
How to reproduce:
// Documentation claims these exist:
ExpoConstants.debuggerHost // Does not exist
ExpoConstants.hostUri // Does not exist
// But actual code uses:
Constants.expoConfig?.hostUri // This is correctResult: Documentation inconsistency misleads developers about the correct API usage
Expected: JSDoc should match the actual implementation and reference Constants.expoConfig?.hostUri per Expo Constants API docs - hostUri is only available in expoConfig during development
2 participants
This fixes #1238
I updated the name of the env variable from
EXPO_PUBLIC_SERVER_ORIGINtoEXPO_PUBLIC_SERVER_URLsince that is the name the docs mentionsPreviously the code checked if
window.locationis defined, which threw an error becausewindowdid not exist, this caused it to fallback to the default URL without checking the env variableThe new code will check if the env variable FIRST and use it if it exists, then check safely for the window and then for the debuggerHost
Summary by CodeRabbit
Bug Fixes
Chores