Skip to content

refactor: fix App.prototype.handler() return type #2908

Closed
timreichen wants to merge 2 commits into
freshframework:mainfrom
timreichen:fix-App-handler-return-type
Closed

refactor: fix App.prototype.handler() return type #2908
timreichen wants to merge 2 commits into
freshframework:mainfrom
timreichen:fix-App-handler-return-type

Conversation

@timreichen

Copy link
Copy Markdown
Contributor

No description provided.

@marvinhagemeister marvinhagemeister left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we should leak this edge case to users. There is no other scenario that I'm aware of where the returned function could return a Response synchronously. We shouldn't expose that internal to users. Did you find more scenarios in which that could happen?

@timreichen

Copy link
Copy Markdown
Contributor Author

I don't think we should leak this edge case to users. There is no other scenario that I'm aware of where the returned function could return a Response synchronously. We shouldn't expose that internal to users. Did you find more scenarios in which that could happen?

No, although middlewares allow both Response and Promise<Response> return values. So in a sense Response | Promise<Response> probably is expected as a return type even if all middlewares are awaited.
I don't see the exposure as a problem in that this is what is being returned.

@marvinhagemeister

Copy link
Copy Markdown
Contributor

Closing as we're not going to land this.

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.

2 participants