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

Use reference types in endpoints #3

Closed
aturon opened this issue Nov 8, 2018 · 7 comments
Closed

Use reference types in endpoints #3

aturon opened this issue Nov 8, 2018 · 7 comments
Labels
blocked An issue that's blocked on upstream changes

Comments

@aturon
Copy link
Collaborator

aturon commented Nov 8, 2018

Currently, there's no way to write a trait bound that corresponds to an async fn that takes borrowed arguments. Because of that restriction, endpoints have to be passed fully owned data, which forces app data to be a cloneable handle.

Once rust-lang/rust#51004 is closed, revisit these APIs (which will also have an impact on the structure of Middleware).

@aturon aturon added the blocked An issue that's blocked on upstream changes label Nov 8, 2018
@aturon aturon changed the title Use reference types in endpoint Use reference types in endpoints Nov 8, 2018
@yoshuawuyts
Copy link
Member

Note from triage: this feature might not even be needed if we end up going with a more layered design in Tide.

@mmrath
Copy link
Contributor

mmrath commented Mar 17, 2019

@yoshuawuyts Is there any notes on what was discussed on this?
I would like to enhance the Cookie support such that it is only computed once per request. That computation is then stored in request extension. Any middleware or request handler can then use this Cookie. Also allow middleware/handlers can modify the cookie. All such modifications can then be written to response from a middleware.

@yoshuawuyts
Copy link
Member

@mmrath if I'm remembering correctly, then by moving the extractors out of Tide's core we can simplify the design enough that this is no longer needed.

The extractors would then become a concept on top of the core API. The exact semantics around that haven't been decided yet, as nailing the core API is a bit more of a priority at the moment.

Does that more or less answer it?

@mmrath
Copy link
Contributor

mmrath commented Mar 21, 2019

Does that more or less answer it?

Yes, somewhat. I will wait for the core api design.

@yoshuawuyts
Copy link
Member

@mmrath speaking of which: #156 is now up! 🎉

@aturon
Copy link
Collaborator Author

aturon commented Apr 10, 2019

Update after #156:

To clarify: we'll still be able to make some API improvements once this compiler limitation is lifted. In particular, right now middleware has to pass a fully owned context downward, which means that if you want to read information out of the context afterward, you have to arrange for that separately.

Ultimately, both middleware and endpoints should work with &mut Context once this issue is unblocked.

@yoshuawuyts
Copy link
Member

We've moved to only taking owned values in endpoints and that seems to be working alright for us. Once we have GATs we may be able to reconsider, but for now it seems fine. Going to go ahead and close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked An issue that's blocked on upstream changes
Projects
None yet
Development

No branches or pull requests

3 participants