Fixes a number of issues where ext:dev:emulator:* did not allow valid extension configurations #3529
Conversation
… extension configurations
| @@ -13,7 +13,7 @@ export function checkResponse(response: string, spec: Param): boolean { | |||
| let valid = true; | |||
| let responses: string[]; | |||
|
|
|||
| if (spec.required && !response) { | |||
| if (spec.required && (response == "" || response == undefined)) { | |||
There was a problem hiding this comment.
This is a little bit ugly, but necessary to cover both the case where a param is completely omitted from the .env file, and the case where a param is set to nothing in the .env file.
There was a problem hiding this comment.
Wait can you help me understand how this code change is any different?
let a;
if(!a) {
console.log("undefined!")
}
a = ""
if(!a) {
console.log("empty string!")
}
=> "undefined!"
=> "empty string!"https://jsfiddle.net/cvyb3fja/
The !response condition seems to capture empty string and undefined value.
There was a problem hiding this comment.
So, looking at this again, I think the real issue is that the typing here is pretty wonky. The YAML parser and .ENV parser we use cast strings that look like numbers or booleans to actual numbers or booleans, and we don't always cast those back to strings before passing them around. In this case, this was breaking if you ever tried to set a param to "false" or "0" - the parsers would cast those to boolean false or number 0, and suddenly !response == true.
If you think this is too hacky, I can try to fix these typing issues in this PR as well.
There was a problem hiding this comment.
It seems like we're trying to exclude the response=false from falling into this if block? Maybe we should explicitly specify that?
i.e. (!response && response !== false)
There was a problem hiding this comment.
YAML parsing is full of weird surprises..
| @@ -13,7 +13,7 @@ export function checkResponse(response: string, spec: Param): boolean { | |||
| let valid = true; | |||
| let responses: string[]; | |||
|
|
|||
| if (spec.required && !response) { | |||
| if (spec.required && (response == "" || response == undefined)) { | |||
There was a problem hiding this comment.
Wait can you help me understand how this code change is any different?
let a;
if(!a) {
console.log("undefined!")
}
a = ""
if(!a) {
console.log("empty string!")
}
=> "undefined!"
=> "empty string!"https://jsfiddle.net/cvyb3fja/
The !response condition seems to capture empty string and undefined value.
… extension configurations (firebase#3529) * fixes a number of issues where ext:dev:emulator:* did not allow valid extension configurations * Adding tests for cases fixed * add changelog * format * pr fixes * correct a comment
Description
Fixes a bunch of issues where the param and spec validation for the extension emulator behaved badly:
I also did some minor modernization and cleanup of these files.
Fixes #2928
Scenarios Tested
I tested a number of extension that Invertase reported were broken on the emulator. I also found some cases where the Storage and auth emulators weren't working correctly with extension - I'm going to try to fix that in a followup.