-
Notifications
You must be signed in to change notification settings - Fork 73
Make this work in Firebase Studio #1077
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: master
Are you sure you want to change the base?
Changes from 8 commits
9282661
3e0dcd6
d773345
fb09148
6164bc2
bfb106d
83e6b48
a0ba027
4ed9a39
08b0e99
fe63dc5
a824de7
b663f91
299e1e7
c1340f6
9a82ea4
6524184
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -234,6 +234,10 @@ async function configFetcher(url: string): Promise<Config> { | |
| // resolve it to IPv6 and connection may fail. | ||
| host = '127.0.0.1'; | ||
| } | ||
| if (key == 'firestore') { | ||
| // If we remapped the host, apply it to the websocket host too. | ||
| value.webSocketHost = value.host; | ||
joehan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| break; | ||
| } else if (listen.address === '::') { | ||
| port = listen.port; | ||
|
|
@@ -242,21 +246,21 @@ async function configFetcher(url: string): Promise<Config> { | |
| // Ditto for IPv6. | ||
| host = '::1'; | ||
| } | ||
| if (key == 'firestore') { | ||
| // If we remapped the host, apply it to the websocket host too. | ||
| value.webSocketHost = value.host; | ||
joehan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| result[key as Emulator] = { | ||
| ...value, | ||
| host, | ||
| port, | ||
| hostAndPort: hostAndPort(host, port), | ||
| }; | ||
| } | ||
|
|
||
| if (result.firestore?.webSocketPort) { | ||
|
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. previously you only remapped the webSocketPort if it was set, do you need to preserve this if statement?
Member
Author
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. I think it's likely irrelevant (webSocketPort is always provided), but added it back in 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. Did you mean to bring this back? |
||
| // Apply the same `host` change above to the WebSocket server. | ||
| result.firestore.webSocketHost = result.firestore.host; | ||
| } | ||
| return result; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -40,7 +40,7 @@ export abstract class RestApi { | |||||
| ) { | ||||||
| const { accessToken } = this.getToken(); | ||||||
|
|
||||||
| const res = await fetch(url, { | ||||||
| const res = await fetchMaybeWithCredentials(url, { | ||||||
| method, | ||||||
| body: data ? JSON.stringify(data) : undefined, | ||||||
| headers: { | ||||||
|
|
@@ -61,7 +61,7 @@ export abstract class RestApi { | |||||
| protected async deleteRequest(url: string) { | ||||||
| const { accessToken } = this.getToken(); | ||||||
|
|
||||||
| const res = await fetch(url, { | ||||||
| const res = await fetchMaybeWithCredentials(url, { | ||||||
| method: 'DELETE', | ||||||
| headers: { | ||||||
| Authorization: 'Bearer ' + accessToken, | ||||||
|
|
@@ -97,7 +97,7 @@ export abstract class RestApi { | |||||
| // Furthermore, setting non-simple headers may cause a CORS pre-flight | ||||||
| // request. RTDB Emulator, for one, doesn't handle those correctly. | ||||||
| // See: https://fetch.spec.whatwg.org/#simple-header | ||||||
| const res = await fetch(url, { method, body }); | ||||||
| const res = await fetchMaybeWithCredentials(url, { method, body }); | ||||||
|
|
||||||
| const text = await res.text(); | ||||||
|
|
||||||
|
|
@@ -115,3 +115,23 @@ function toFormData(file: File) { | |||||
| formData.append('upload', file); | ||||||
| return formData; | ||||||
| } | ||||||
|
|
||||||
| export function isCloudWorkstation(url: string) { | ||||||
| return url.includes('.cloudworkstations.dev'); | ||||||
| } | ||||||
|
|
||||||
| // This is a very minimal wrapper around fetch that lets us more easily make | ||||||
| // broad changes to API calls. | ||||||
| // Previously, we had a ton a raw fetch calls throughout the codebase, so any broad changes needed to be made all over the place. | ||||||
| export async function fetchMaybeWithCredentials( | ||||||
| url: string, | ||||||
| opts?: RequestInit | ||||||
| ): Promise<Response> { | ||||||
| if (!opts) { | ||||||
| opts = {}; | ||||||
| } | ||||||
| if (isCloudWorkstation(url)) { | ||||||
| opts.credentials = 'include'; | ||||||
|
||||||
| opts.credentials = 'include'; | |
| opts = {...opts, credentials: 'include'}; |
It will handle undefined without Line 130-132 and it avoids modifying the argument
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.
Are we merging this for an official release?
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.
No, we'll wait for the real release - adding a note to the descriptio n