-
Notifications
You must be signed in to change notification settings - Fork 0
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
Refactor model call query params #41
Refactor model call query params #41
Conversation
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.
Much cleaner than we originally had, thank you!
src/models/model.ts
Outdated
function objectToURLSearchParams(object: Record<string, unknown>) { | ||
const params = new URLSearchParams(); | ||
for (const key in object) { | ||
if (typeof object[key] === 'object') { |
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.
Minor – Let's use isPureObject
from utils, instead of the typeof check here. Probably doesn't matter, but this way "null" falls through to the next case and we can do a null or undefined check.
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.
And will the isPureObject
check catch arrays too?
Right now, checking for object
types we are stringifying object and arrays, and converting null
to "null"
.
However, we could separate these three cases in three conditionals to make it easier to read
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.
It would read as something like this
if (isPureObject(object[key])) {
params.append(key, JSON.stringify(object[key]));
}
else if (Array.isArray(object[key])) {
params.append(key, JSON.stringify(object[key]));
}
else if (object[key] === null) {
params.append(key, 'null');
}
else if (object[key] !== undefined) {
params.append(key, String(object[key]));
}
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.
Something Im not sure about is how we want to treat null
values here
…alls and switch conditionals
Following up on the conversation here: #34 (comment) I've created this PR with a refactor proposal of this internal part, to reduce the amount of type casting