feat: parameter deserialization #3116
Conversation
|
View your CI Pipeline Execution ↗ for commit 4d0e172.
☁️ Nx Cloud last updated this comment at |
github-actions
bot
added
enhancement
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
sdo-1A
force-pushed
the
feat/param-deserialization
branch
from
8a7ca39 to
50908c7
Compare
| it('should correctly deserialize query parameters of type array', () => { | ||
| expect(deserializeQueryParams({ idArray: 'idArray=3&idArray=4&idArray=5' }, { idArray: { explode: true, style: 'form', paramType: 'array' } })).toEqual({ idArray: ['3', '4', '5'] }); | ||
| expect(deserializeQueryParams({ idArray: 'idArray=3,4,5' }, { idArray: { explode: false, style: 'form', paramType: 'array' } })).toEqual({ idArray: ['3', '4', '5'] }); | ||
| expect(deserializeQueryParams({ idArray: 'should-not-work' }, { idArray: { explode: true, style: 'spaceDelimited', paramType: 'array' } })).toEqual({ idArray: undefined }); |
There was a problem hiding this comment.
should this return undefined or throw an error?
There was a problem hiding this comment.
this should return undefined, we don't throw errors for non-supported use cases
There was a problem hiding this comment.
I would expect a function like this to throw when the format is not recognized, but I'm ok to return undefined
| import { | ||
| CLOSING_SQUARE_BRACKET_URL_CODE, | ||
| OPENING_SQUARE_BRACKET_URL_CODE, | ||
| ParamSerialization, |
There was a problem hiding this comment.
| ParamSerialization, | |
| type ParamSerialization, |
?
| acc[queryParamName as keyof T] = deserializedValue; | ||
| } | ||
| return acc; | ||
| }, {} as { [p in keyof T]: SupportedParamType }); |
There was a problem hiding this comment.
the type is a bit incorrect as this stage (accept if T is {}).
Maybe it would be more something like:
| }, {} as { [p in keyof T]: SupportedParamType }); | |
| }, {} as Record<string, SupportedParamType>) as { [p in keyof T]: SupportedParamType }; |
There was a problem hiding this comment.
Isn't it fine to accept {} ? Because we have conditions in the functions to avoid unsupported use cases so it possible that we return an empty object
If I change the type to Record<string, SupportedParamType>, we could not return an empty object {}
There was a problem hiding this comment.
{ [p in keyof T]: SupportedParamType } means that all fields of the object T should be present in your object. So if T has more than 1 field, the code {} as { [p in keyof T]: SupportedParamType } is wrong.
The purpose is not to change the return type (which remains { [p in keyof T]: SupportedParamType }) but the acc type to avoid wrong typing.
| acc[pathParamName as keyof T] = deserializedValue; | ||
| } | ||
| return acc; | ||
| }, {} as { [p in keyof T]: SupportedParamType }); |
There was a problem hiding this comment.
Same comment as above:
| }, {} as { [p in keyof T]: SupportedParamType }); | |
| }, {} as Record<string, SupportedParamType>); |
sdo-1A
force-pushed
the
feat/param-deserialization
branch
from
50908c7 to
4d0e172
Compare
Proposed change
Provide deserialization functions as a tool to deserialize query and path parameters.
This PR was created as a split of PR #3076
Related issues
- No issue associated -