-
Notifications
You must be signed in to change notification settings - Fork 17
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
API params serializer #449
Conversation
import { decamelizeKeys } from 'humps'; | ||
import { stringify } from 'qs'; | ||
|
||
export function serializeParams(params: Record<string, string | number | 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.
Maybe these types are too limited, it should be able to handle nested arrays and objects (for example when making requests to Ransack
)
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.
True, changed it to object
. Hope I didn't overcompensate
@@ -16,6 +17,7 @@ const api = axios.create({ | |||
'Accept': 'application/json', | |||
'X-CSRF-Token': csrfToken(), | |||
}, | |||
paramsSerializer: (params) => serializeParams(params), |
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 this could be simplified to paramsSerializer: serializerParams
since it's passing the same argument. Or serializerParams
could be renamed to paramsSerializer
to use the shorthand.
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.
Nice, I'll go with the shorthand 👌
b86fe7f
to
d1d6993
Compare
export function paramsSerializer(params: object) { | ||
const decamelizedParams = decamelizeKeys(params); | ||
|
||
return stringify(decamelizedParams, { arrayFormat: 'repeat' }); |
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.
Sorry, didn't see this before. I think it has to be brackets
here to get the usual array[]=1&array[]=2
format rails uses
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.
Oh, good catch. I thought it didn't matter but never tested it 😬
d1d6993
to
dfc1e9a
Compare
Context
Potassium-generated applications use APIs to send and receive data between their backend and their frontend. Since both sides use different case conventions for hash keys, a conversion is required for each data exchange.
In #412, an API client was added to the frontend that included case conversion for request bodies. But request query params still don't have case conversion, so it's implemented wherever necessary, usually resulting in code repetition.
What has been done
Summary:
A params serializer has been added to the frontend API client that converts the params casing as needed.
To implement this, a query params util was created with one feature: converting an object into a valid URL query string, with keys in snake_case.
Commits:
qs
dependency to the frontend, to handle the conversion of objects into valid query params.query-params
util, to take a JavaScript object and return a valid URL query string with snake_case keys.Extra info
The qs library was used for stringifying params because it was suggested in Axios docs and looks well maintained. If we don't want to add an extra dependency, it could be replaced by a custom solution, but it will probably be less robust.