-
Notifications
You must be signed in to change notification settings - Fork 7
Bug 2001078 - Use generic _fetch to initiate post and get requests in ConsoleClient #270
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
| // TODO: Bug 2001078 - ConsoleClient shouldn't use the | ||
| // _get function to initiate POST requests | ||
| return this._get(path, "POST"); | ||
| async _post(path, jsonBody) { |
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.
Note this is changing the signature, so old callers are passing in undefined.
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.
I guess the simplest is just to give this a default and then the other code can stay as it is.
| const res = await fetch(url, { | ||
| method, | ||
| headers, | ||
| body: jsonBody === null ? undefined : JSON.stringify(jsonBody), |
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.
I think here you will try to stringify undefined.
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.
I just re-checked the behavior of default parameters because I wasn't sure anymore neither:
Default function parameters allow named parameters to be initialized with default values if no value or undefined is passed.
So if undefined is passed to _fetch for jsonBody, then it's initialized with the default value null and we won't pass is to JSON.stringify.
| const headers = new Headers({}); | ||
| const accessToken = await this.getAccessToken(); | ||
| headers.set("Authorization", `Bearer ${accessToken}`); | ||
| if (jsonBody !== null) { |
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.
This will trigger even if jsonBody is undefined.
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.
jsonBody can't be undefined, see explanation below.
No description provided.