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.

Sign up for GitHub

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

Jump to bottom

Static files #415

Merged
merged 1 commit into from
Apr 21, 2020
Merged

Static files #415

merged 1 commit into from
Apr 21, 2020

Conversation

yoshuawuyts
Copy link
Member

Init static file serving support. This depends on #414.

#[async_std::main]
async fn main() -> tide::Result<()> {
    let mut app = tide::new();
    app.at("/").serve_dir("/public");
    app.listen("127.0.0.1:8080").await?;
    Ok(())
}

@yoshuawuyts yoshuawuyts mentioned this pull request Mar 5, 2020
Closed
pickfire
pickfire reviewed Mar 26, 2020
View reviewed changes
src/server/serve_dir.rs Outdated Show resolved Hide resolved
@pickfire
Copy link
Contributor

@yoshuawuyts Is there any reason why the original body builder method is changed to set_body method separately? I personally don't think it looks nice but probably there is a reason behind it.

@yoshuawuyts
Copy link
Member Author

@pickfire this is more of a temporary name than anything else. Ideally we could get closer to http-types' way of constructing requests and responses but this is a stopgap in the meantime.

@pickfire
Copy link
Contributor

@yoshuawuyts It seemed similar to the one in http-types. Is there any reason why it uses set_body rather than body_mut or body? Currently, http-types have quite a few *_body and it does seemed a bit weird for me.

@yoshuawuyts yoshuawuyts marked this pull request as ready for review April 18, 2020 22:42
goto-bus-stop
goto-bus-stop reviewed Apr 20, 2020
View reviewed changes
Copy link
Member

@goto-bus-stop goto-bus-stop left a comment

Choose a reason for hiding this comment

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

i haven't looked at tide much but this part looks good :)

src/server/route.rs Outdated Show resolved Hide resolved
@yoshuawuyts
Copy link
Member Author

@pickfire http-types has the following body methods: docs.rs/http-types?search=body. There are currently four methods:

  • set_body to set a body.
  • swap_body to replace a body with a different body through a reference.
  • replace_body to replace body with a different body through ownership.
  • body_string to read the body as a string (we plan to make this respect encodings as well).

I suspect we'll be adding more methods to request and response for reading the body as various types. Likely candidates include: json, form, and bytes. But setting the body will almost exclusively go through set_body which takes Into<Body> which has defined conversions from many types. This should make setting a body overall a smooth process, and is the reason why I chose to go with set_body in this patch as well (also note: none of the existing methods worked; I actually spent a good amount of time debugging why filed with a known length were getting chunk-encoded).

rkusa
rkusa reviewed Apr 20, 2020
View reviewed changes
Copy link
Contributor

@rkusa rkusa left a comment

Choose a reason for hiding this comment

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

I was reading the code out of curiosity and wrote down some thoughts that came up while doing so, I hope you don’t mind.

src/response/into_response.rs Outdated Show resolved Hide resolved
src/response/into_response.rs Outdated Show resolved Hide resolved
src/server/serve_dir.rs Outdated Show resolved Hide resolved
src/server/route.rs Show resolved Hide resolved
@pickfire
Copy link
Contributor

@yoshuawuyts But why do we need so many different methods? Couldn't that be just body (shared reference) and body_mut (unique reference)?

@yoshuawuyts
Copy link
Member Author

@pickfire in the http-types model both Request and Response provide direct access to the body and headers. This has some similarities to Ruby's net::http model.

Personally I really dislike the body/body_mut distinction since it leads to patterns like these:

*req.body_mut() = MyBody;

Std doesn't use this pattern anywhere, and instead prefers the distinction between body and set_body. But where possible (e.g. in io::Cursor) it may just forward trait impls such as Read/Write directly and only provide access to the inner value through an into_inner method. This is the model we've decided to follow instead since it provides a reduced surface area overall.

The convenience methods I mentioned earlier are really only to move beyond the basics. Being able to automatically deserialize from JSON while respecting encodings is pretty hard to do, and providing a shorthand for this is useful. This goes for many of the other methods as well.

@yoshuawuyts
Add static file serving
f3af561 
This depends on #414

Add serve_dir method to Route

Update serve_dir.rs

Adjust methods so body can correctly be sent along

tweak static file serving

simplify internals

cargo fmt & fix tests

fix all tests

cargo fmt

Fix merge conflicts with master

undo bonus changes

Fix static path prefix stripping err
@yoshuawuyts yoshuawuyts merged commit 0309d95 into master Apr 21, 2020
@yoshuawuyts yoshuawuyts deleted the static-files branch April 21, 2020 11:30
@yoshuawuyts yoshuawuyts mentioned this pull request Apr 21, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pickfire pickfire pickfire left review comments

@goto-bus-stop goto-bus-stop goto-bus-stop left review comments

@rkusa rkusa rkusa left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yoshuawuyts @pickfire @rkusa @goto-bus-stop