[Experiment] Adds "firebase shell" command. #1865
Conversation
|
@bkendall I put this behind a preview flag because I think it's pretty safe to merge even if it's still fairly experimental. |
src/commands/shell.ts
Outdated
| return Object.assign({ __id__: snap.id }, snap.data()); | ||
| } | ||
|
|
||
| module.exports = new Command("shell [script_path]") |
There was a problem hiding this comment.
export default new Command should be okay here
src/commands/shell.ts
Outdated
| } | ||
| } | ||
|
|
||
| function runRepl(sandbox: Context) { |
There was a problem hiding this comment.
Can return types be added where necessary? any warnings can be ignored, but return types are useful.
| const script = new Script(readFileSync(scriptPath, { encoding: "utf8" }), { | ||
| filename: scriptPath, | ||
| }); | ||
| return await Promise.resolve(script.runInContext(sandbox)); |
There was a problem hiding this comment.
Why the additional wrapping? Also, does it return something worthwhile? Maybe just await the script and remove the return.
| return await Promise.resolve(script.runInContext(sandbox)); | |
| return await script.runInContext(sandbox); |
(I'm seeing a comment regarding the wrapping below - if it's the same condition, let's add the same comment here)
src/commands/shell.ts
Outdated
| import { readFileSync, existsSync } from "fs"; | ||
| import fetch, { Response } from "node-fetch"; | ||
| import { DocumentSnapshot, Firestore, QuerySnapshot } from "@google-cloud/firestore"; | ||
| import api = require("../api"); |
There was a problem hiding this comment.
Can you import the other 2 values needed in line 6 above where api is being import'd?
src/commands/shell.ts
Outdated
| const replServer = start({ | ||
| prompt: `${yellow("firebase")}${red(">")} `, | ||
| eval: shellEval, | ||
| // writer: replWriter, |
| replServer.on("exit", resolve); | ||
| process.on("SIGINT", resolve); |
There was a problem hiding this comment.
Does SIGINT cause exit to fire too?
There was a problem hiding this comment.
I...don't remember why I have this here but I remember it's necessary. I think it catches a weird state where you had to hit Ctrl+C multiple times or something.
| client_id: api.clientId, | ||
| client_secret: api.clientSecret, | ||
| refresh_token: getRefreshToken(), | ||
| } as any, |
There was a problem hiding this comment.
The Firestore library doesn't have a type for this? I thought that one carried its own types. It'd be nice to not cast something as any if we can help it.
There was a problem hiding this comment.
It's a hack to allow user credentials, which is an undocumented but technically supported feature. See this issue which I will add in a comment.
Implements a
firebase shellcommand that is pre-initialized with an Admin SDK to perform scripted administrative tasks easily.