Skip to content
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

ref: remove usage of express-serve-static-core types (ParamsDictionary, Query) #1848

Merged
merged 19 commits into from
Jun 1, 2021

Conversation

karrui
Copy link
Contributor

@karrui karrui commented May 10, 2021

Problem

This PR creates a new ControllerHandler interface that extends from express#RequestHandler interface but replaces all the any casts to unknown, for stricter typing. With that, the dependency @types/express-serve-static-core is no longer needed and removed from the application.

This PR is however blocked by #1854 as submitEncryptModeForm has no typings and will now throw a compiler error due to the stricter unknown type (from any). Will be merged in when that issue is resolved.

Closes #948

Solution

Features:

  • add ControllerHandler type to cast unknowns to RequestHandler
  • replace RequestHandler in controllers with ControllerHandler

Improvements:

  • loosen Request type used in utils/request.ts
  • remove dependency on @types/express-serve-static-core by updating types used

Dependencies:

  • remove @types/express-serve-static-core

Copy link
Contributor

@tshuli tshuli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

src/app/modules/examples/examples.controller.ts Outdated Show resolved Hide resolved
src/app/utils/request.ts Outdated Show resolved Hide resolved
@karrui karrui added the blocked blocked by another issue label May 11, 2021
loosening allows any type that extends from express#Request to be passed to the utility functions. This does not matter since the utilities do not use the types params that are being supplied to the interface
also add some missing param types, remove some typecasts
@karrui
Copy link
Contributor Author

karrui commented May 11, 2021

Will reopen when types are fixed in #1854

@karrui karrui closed this May 11, 2021
@karrui karrui reopened this May 25, 2021
@karrui karrui marked this pull request as draft May 25, 2021 11:06
# Conflicts:
#	src/app/modules/form/admin-form/admin-form.controller.ts
#	src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts
#	src/app/modules/verification/verification.controller.ts
# Conflicts:
#	package.json
#	src/app/modules/verification/verification.controller.ts
@karrui karrui removed the blocked blocked by another issue label May 31, 2021
@karrui karrui marked this pull request as ready for review May 31, 2021 03:13
@karrui karrui requested review from tshuli and mantariksh May 31, 2021 03:35
@mantariksh mantariksh merged commit 8ed8fc7 into develop Jun 1, 2021
@yong-jie yong-jie mentioned this pull request Jun 1, 2021
@karrui karrui deleted the ref/remove-paramsdictionary branch August 17, 2021 02:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use never instead of ParamsDictionary in middleware
3 participants