Skip to content

Moving the emulator UI server to firebase-tools.#7897

Merged
christhompsongoogle merged 16 commits intomasterfrom
servertsmove
Nov 13, 2024
Merged

Moving the emulator UI server to firebase-tools.#7897
christhompsongoogle merged 16 commits intomasterfrom
servertsmove

Conversation

@christhompsongoogle
Copy link
Copy Markdown
Contributor

@christhompsongoogle christhompsongoogle commented Nov 1, 2024

Move the server.ts file to firebase-tools so that the emulator UI can be served in-process with the firebase CLI.

Some questions are inline with the code (marked with "FIXME") for second opinions

@christhompsongoogle christhompsongoogle linked an issue Nov 7, 2024 that may be closed by this pull request
Copy link
Copy Markdown
Member

@yuchenshi yuchenshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approach looks good. One nit inline. Please run npm run fmt to fix linter errors

Comment on lines +80 to +82
app.get("*", (_, res) => {
res.sendFile(path.join(webDir, "index.html"));
});

Check failure

Code scanning / CodeQL

Missing rate limiting

This route handler performs [a file system access](1), but is not rate-limited.
Copy link
Copy Markdown
Member

@joehan joehan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nits, but LGTM overall

@christhompsongoogle christhompsongoogle enabled auto-merge (squash) November 13, 2024 23:35
@christhompsongoogle christhompsongoogle merged commit 01ad9e5 into master Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants