-
Notifications
You must be signed in to change notification settings - Fork 1k
Include appId in WebConfig #3267
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
Conversation
Bump @kmcnellis can you take a quick look at this one? |
src/fetchWebSetup.ts
Outdated
let hostingAppId: string | undefined = undefined; | ||
try { | ||
const sites = await hostingApiClient.get<ListSitesResponse>(`/projects/${projectId}/sites`); | ||
const defaultSite = sites.body.sites.find((s) => s.type === "DEFAULT_SITE"); |
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.
Does the emulator support non-default sites?
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.
At the moment, no. One day!
|
||
// Get the web app config for the appId, or use the '-' special value if the appId is not known | ||
const appId = hostingAppId || "-"; | ||
const res = await apiClient.get<WebConfig>(`/projects/${projectId}/webApps/${appId}/config`); |
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.
Probably worth a try/catch around this
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.
The only caller has a try/catch that does the right thing. I'm gonna leave it rather than increase the scope of this refactor.
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.
If it's a bogus project ID, should we make a bogus config object as well? Or does firebaseConfig = undefined allow everything to still work in the emulator? I don't think it should be a hard requirement to use a real project id with the emulators
@kmcnellis we're working on some emulator-suite-wide handling for bogus project IDs but right now this maintains the status quo: the emulator will start in a somewhat degraded mode with a bogus project ID. |
Description
Fixes #2798
Scenarios Tested
Fetch __init.js with a site<>webapp linked:
Fetch __init.js with a totally bogus project ID:
Sample Commands