Skip to content

Commit 70235a6

Browse files
authored
Merge pull request #65 from amalej/fix_fbtools_v15
fix issue where tool does not work with firebase-tools v15
2 parents 8bf144d + 5976ad1 commit 70235a6

5 files changed

Lines changed: 85 additions & 31 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
<!-- ADD CHANGES HERE -->
22

3+
Unreleased
4+
5+
- Fixed issue where Onfire CLI cannot parse commands on firebase-tools v15
6+
- This is due to the lazy loading feature preventing all commands from being loaded
7+
38
v1.3.2
49

510
- Added feature to list folders/files in current path to make it easier to use

src/firebase-cmd.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
spawnSync,
88
} from "child_process";
99
import spawn from "cross-spawn";
10+
import { writeFileSync } from "fs";
1011

1112
const COMMAND_TIMEOUT = 10000;
1213

@@ -251,12 +252,15 @@ export class FirebaseCommands {
251252
});
252253

253254
child.on("exit", () => {
254-
if (childBufferString === "ENOENT") {
255-
throw new Error(
256-
`Firebase Tools module not found. Install using 'npm install -g firebase-tools'`
255+
this.loadModuleChildProcess?.kill();
256+
this.loadModuleChildProcess = null;
257+
if (childBufferString.includes("ERROR__")) {
258+
const message = childBufferString.replace(
259+
"ERROR__",
260+
"Error loading module: "
257261
);
262+
throw new Error(message);
258263
}
259-
this.loadModuleChildProcess = null;
260264
try {
261265
res(JSON.parse(childBufferString));
262266
} catch (_) {
@@ -266,7 +270,6 @@ export class FirebaseCommands {
266270
});
267271

268272
child.send(`${rootPath}/firebase-tools`);
269-
// this.loadModuleChildProcess.kill();
270273
});
271274

272275
// await new Promise((res, rej) => setTimeout(res, 5000));

src/module-loader.js

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,54 @@ function stringify(obj) {
1515
return str;
1616
}
1717

18+
function determineCommandList(map, parentKey = null, depth = 0) {
19+
if (depth > 10) return []
20+
const commandList = []
21+
if (map) {
22+
for (let [key, value] of Object.entries(map)) {
23+
if (key === "load") continue
24+
if (typeof value?.load === "function") {
25+
if (!parentKey) {
26+
commandList.push(key)
27+
} else {
28+
commandList.push([parentKey, key].join(":"))
29+
}
30+
}
31+
32+
if (!parentKey) {
33+
commandList.push(...determineCommandList(value, key, depth + 1))
34+
} else {
35+
commandList.push(...determineCommandList(value, [parentKey, key].join(":"), depth + 1))
36+
}
37+
}
38+
}
39+
40+
return commandList.map((e) => e.toLowerCase())
41+
}
42+
1843
process.on('message', function (path) {
1944
try {
2045
const client = require(path); // Load the Firebase Tools module
21-
const configStr = stringify(client) // Removes circular reference and convert to string
46+
let configStr = stringify(client) // Removes circular reference and convert to string
47+
48+
const clientObj = JSON.parse(configStr)
49+
if (clientObj.cli.commands.length === 0) {
50+
/**
51+
* Due to firebase-tools v15 introducing lazy laoding of commands
52+
* we would need to run `getCommand([command_name])` for each command
53+
* to register each command
54+
*/
55+
const commands = determineCommandList(client)
56+
for (let command of commands) {
57+
require(path).getCommand(command)
58+
}
59+
60+
const clientWithLoadedCommands = require(path);
61+
configStr = stringify(clientWithLoadedCommands)
62+
}
63+
2264
process.send(configStr); // Send to main process
23-
} catch (_) {
24-
process.send('ENOENT'); // Send to main process
65+
} catch (err) {
66+
process.send(`ERROR__${err}`); // Send to main process
2567
}
2668
});

src/onfire-cli.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1034,23 +1034,25 @@ export class OnFireCLI extends CommandLineInterface {
10341034
} else {
10351035
this.firebaseCommands = cmdConfig;
10361036
}
1037+
this.attachCustomCommands();
1038+
this.savedConfig["firebaseCommands"] = this.firebaseCommands;
10371039
} else {
10381040
this.firebaseCommands = this.savedConfig["firebaseCommands"];
10391041
this.firebaseCli
10401042
.getCommandConfig()
10411043
.then((firebaseCommands) => {
10421044
if (firebaseCommands !== null) {
10431045
this.firebaseCommands = firebaseCommands;
1044-
this.savedConfig["firebaseCommands"] = this.firebaseCommands;
10451046
this.attachCustomCommands();
1047+
this.savedConfig["firebaseCommands"] = this.firebaseCommands;
1048+
} else {
1049+
throw new Error("Could not load firebase-tools commands");
10461050
}
10471051
})
1048-
.catch((_) => {
1049-
// No action needed since we can use the cached firebaseCommands
1052+
.catch((err) => {
1053+
throw err;
10501054
});
10511055
}
1052-
this.attachCustomCommands();
1053-
this.savedConfig["firebaseCommands"] = this.firebaseCommands;
10541056
}
10551057

10561058
async init() {

tests/onfire-cli.test.ts

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ class MockOnFireCLI extends OnFireCLI {
7373
}
7474

7575
const appdistribution = {
76-
distributeLen: 10,
76+
distributeLen: 20,
7777
testers: {
7878
addLen: 5,
7979
removeLen: 5,
@@ -94,22 +94,24 @@ describe("Test loading of Firebase commands", () => {
9494
it("Should load 'appdistribution:distribute' command", async () => {
9595
const firebaseCommands = onfireCLI._getFirebaseCommands();
9696
expect(firebaseCommands["appdistribution:distribute"].description).toEqual(
97-
"upload a release binary"
97+
"upload a release binary and optionally distribute it to testers and run automated tests"
9898
);
9999
});
100100

101101
it("Should load 'appdistribution:testers:add' command", async () => {
102102
const firebaseCommands = onfireCLI._getFirebaseCommands();
103103
expect(firebaseCommands["appdistribution:testers:add"].description).toEqual(
104-
"add testers to project (and possibly group)"
104+
"add testers to project (and App Distribution group, if specified via flag)"
105105
);
106106
});
107107

108108
it("Should load 'appdistribution:testers:remove' command", async () => {
109109
const firebaseCommands = onfireCLI._getFirebaseCommands();
110110
expect(
111111
firebaseCommands["appdistribution:testers:remove"].description
112-
).toEqual("remove testers from a project (or group)");
112+
).toEqual(
113+
"remove testers from a project (or App Distribution group, if specified via flag)"
114+
);
113115
});
114116

115117
it("Should load 'exit' command", async () => {
@@ -147,6 +149,15 @@ describe("Run simple commands", () => {
147149
"--testers-file",
148150
"--groups",
149151
"--groups-file",
152+
"--test-devices",
153+
"--test-devices-file",
154+
"--test-username",
155+
"--test-password",
156+
"--test-password-file",
157+
"--test-username-resource",
158+
"--test-password-resource",
159+
"--test-case-ids",
160+
"--test-case-ids-file",
150161
"--project",
151162
]);
152163
});
@@ -471,20 +482,20 @@ describe("Test getting rendering list", () => {
471482
expect(renderMessage.length).toEqual(5);
472483
});
473484

474-
it("Should show that the selected command in index [0] is 'appdistribution:distribute -> upload a release binary'", () => {
485+
it("Should show that the selected command in index [0] is 'appdistribution:distribute -> upload a release binary and optionally distribute it to testers and run automated tests'", () => {
475486
const renderMessage = onfireCLI._getCommandsToRender();
476-
const cmdLabel = `-> upload a release binary`;
487+
const cmdLabel = `-> upload a release binary and optionally distribute it to testers and run automated tests`;
477488
expect(renderMessage[0]).toEqual(
478489
`${cli._textCyan(cli._textBold(">"))} ${cli._textGreen(
479490
cli._textBold("appdistribution:distribute")
480491
)} ${cli._textGreen(cmdLabel)}\x1b[K`
481492
);
482493
});
483494

484-
it("Should show that the unselected command in index [1] is 'appdistribution:testers:add -> add testers to project (and possibly group)'", () => {
495+
it("Should show that the unselected command in index [2] is 'appdistribution:testers:add -> add testers to project (and App Distribution group, if specified via flag)'", () => {
485496
const renderMessage = onfireCLI._getCommandsToRender();
486-
const cmdLabel = `-> add testers to project (and possibly group)`;
487-
expect(renderMessage[1]).toEqual(
497+
const cmdLabel = `-> add testers to project (and App Distribution group, if specified via flag)`;
498+
expect(renderMessage[2]).toEqual(
488499
` ${cli._textBold("appdistribution:testers:add")} ${cmdLabel}\x1b[K`
489500
);
490501
});
@@ -542,15 +553,6 @@ describe("Test getting rendering list", () => {
542553
)} ${cli._textGreen(cmdLabel)}\x1b[K`
543554
);
544555
});
545-
546-
it("Should show that the unselected option in index [1] is 'experimental:functions:shell -> launch full Node shell with emulated functions. (Alias for `firebase functions:shell.)'", () => {
547-
const renderMessage = onfireCLI._getCommandsToRender();
548-
const cmdLabel =
549-
"-> launch full Node shell with emulated functions. (Alias for `firebase functions:shell.)";
550-
expect(renderMessage[1]).toEqual(
551-
` ${cli._textBold("experimental:functions:shell")} ${cmdLabel}\x1b[K`
552-
);
553-
});
554556
});
555557
});
556558

0 commit comments

Comments
 (0)