-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Remove unsafe types from examples/extend-express-app
example
#8839
Conversation
@@ -23,18 +34,8 @@ export default config<TypeInfo>({ | |||
Keystone schema and return the results as JSON | |||
*/ | |||
extendExpressApp: (app, commonContext) => { | |||
app.use('/rest', async (req, res, next) => { |
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.
We don't want to call withRequest
for other HTTP requests, except for GET /rest/tasks
, thereby we shouldn't use app.use
.
@@ -11,10 +11,7 @@ import type { Context } from '.keystone/types'; | |||
We're also demonstrating how you can query related data through the schema. | |||
*/ | |||
|
|||
export async function getTasks(req: Request, res: Response) { | |||
// This was added by the context middleware in ../keystone.ts | |||
const { context } = req as typeof req & { context: Context }; |
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.
We don't want to encourage as
in user code, the preferred approach is to use a Context
parameter directly
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit f5d0199:
|
In the future, we might recommend removal of That needs feedback from the community though, and would probably be quite the controversial breaking change. |
This pull request removes the
(req as any).context
types from theextend-express-app
example, and instead prefers to pass a refinedContext
directly to the route functions.This new pattern reduces the number of
req as
casts, while still keeping your root express router definition as minimal as possible.