-
Notifications
You must be signed in to change notification settings - Fork 410
fix: url creation #1240
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: url creation #1240
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@uploadthing/expo": patch | ||
| --- | ||
|
|
||
| Fix URL construction - Previously the code checked if window.location is defined, which threw an error because window did not exist, this caused it to fallback to the default URL without checking the env variable The 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 |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,9 +16,9 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @example "/api/uploadthing" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @example "https://www.example.com/api/uploadthing" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * If relative, host will be inferred from either the `EXPO_PUBLIC_SERVER_ORIGIN` environment variable or `ExpoConstants.hostUri` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * 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" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| url?: URL | string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -55,17 +55,18 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let url = new URL("http://localhost:8081/api/uploadthing"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| url = new URL( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| initOpts?.url ?? "/api/uploadthing", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| typeof window.location !== "undefined" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? window.location.origin | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : (process.env.EXPO_PUBLIC_SERVER_ORIGIN ?? `http://${debuggerHost}`), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| initOpts?.url ?? "/api/uploadthing", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+59
to
70
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Restore support for This change drops the old env key entirely, so any existing app that already had - 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
JSDoc comments reference
ExpoConstants.debuggerHostbut the actual code usesConstants.expoConfig?.hostUri, creating documentation inconsistency.View Details
Analysis
JSDoc comments reference incorrect
ExpoConstantspropertiesWhat fails: JSDoc documentation in
packages/expo/src/index.tsreferences non-existentExpoConstants.debuggerHostandExpoConstants.hostUriproperties, while the actual implementation correctly usesConstants.expoConfig?.hostUriHow to reproduce:
Result: Documentation inconsistency misleads developers about the correct API usage
Expected: JSDoc should match the actual implementation and reference
Constants.expoConfig?.hostUriper Expo Constants API docs -hostUriis only available inexpoConfigduring development