Closed ghost closed 3 years ago
Merging #213 (af0e74b) into master (b67dcdb) will not change coverage. The diff coverage is
n/a
.
@@ Coverage Diff @@
## master #213 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 4 4
=========================================
Hits 4 4
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update b67dcdb...af0e74b. Read the comment docs.
I don't know TS that well... what does this change get us specifically. Like not from a type soundness standpoint but what functional change does this get us?
Currently there is:
interface Paging {
limit: number;
page: number;
}
let paging = Joi.object<Paging>({
limit: Joi.number();
page: Joi.number();
});
router.get('/', celebrate({ query: paging }), async (req, res, next) => {
const query = req.query; // type QueryString.ParsedQS (default from express)
})
In order to have correct typing, one must write:
router.get('/', celebrate<ParamsDictionary, any, any, Paging>({ query: paging }), async (req, res, next) => {
const query = req.query; // type Paging
})
This a boring notation, even more because we need put all 4 generic values just to type the generic query type.
With my changes, the first notation works and we have:
router.get('/', celebrate({ query: paging }), async (req, res, next) => {
const query = req.query; // type Paging
})
Because type is inferred from arguments
@alburkerk are you still interested in landing this PR?
Closing due to inactivity.
Related issue: https://github.com/arb/celebrate/issues/212