Skip to content

Commit

Permalink
refactor: minor code improvements (#110)
Browse files Browse the repository at this point in the history
* Remove unecessary type annotations

* Initialize let bindings upon declaration

* Pass u16 by value instead of by reference

* Clean up HttpRequest builder logic in conv_req.rs

Removed some unnecessary comments and rewrote repeated reassignments to
method chaining.

* Remove unecessary type annotations in `let mut`
  • Loading branch information
Asha20 committed Jan 16, 2022
1 parent b638d25 commit 2c0d344
Show file tree
Hide file tree
Showing 23 changed files with 99 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ async fn real_main() -> i32 {
// Build that error page as the server does
let err_page_str = build_error_page(
"",
&err_code_to_build_for,
err_code_to_build_for,
"",
None,
&error_pages,
Expand Down
4 changes: 2 additions & 2 deletions examples/basic/.perseus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,11 @@ pub fn run() -> Result<(), JsValue> {
initial_container.set_attribute("style", "display: none;").unwrap();
// Hydrate the error pages
// Right now, we don't provide translators to any error pages that have come from the server
error_pages.render_page(&url, &status, &err, None, &container_rx_elem);
error_pages.render_page(&url, status, &err, None, &container_rx_elem);
} else {
// This is an error from navigating within the app (probably the dev mistyped a link...), so we'll clear the page
container_rx_elem.set_inner_html("");
error_pages.render_page("", &404, "not found", None, &container_rx_elem);
error_pages.render_page("", 404, "not found", None, &container_rx_elem);
}
},
};
Expand Down
2 changes: 1 addition & 1 deletion examples/fetching/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ pub async fn get_build_state(
_locale: String,
) -> RenderFnResultWithCause<IndexProps> {
// We'll cache the result with `try_cache_res`, which means we only make the request once, and future builds will use the cached result (speeds up development)
let body: String = perseus::cache_fallible_res(
let body = perseus::cache_fallible_res(
"ipify",
|| async {
// This just gets the IP address of the machine that built the app
Expand Down
12 changes: 4 additions & 8 deletions packages/perseus-actix-web/src/conv_req.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,15 @@ use perseus::{HttpRequest, Request};
/// Converts an Actix Web request into an `http::request`.
pub fn convert_req(raw: &actix_web::HttpRequest) -> Result<Request, Error> {
let mut builder = HttpRequest::builder();
// Add headers one by one

for (name, val) in raw.headers() {
// Each method call consumes and returns `self`, so we re-self-assign
builder = builder.header(name, val);
}
// The URI to which the request was sent
builder = builder.uri(raw.uri());
// The method (e.g. GET, POST, etc.)
builder = builder.method(raw.method());
// The HTTP version used
builder = builder.version(raw.version());

builder
.uri(raw.uri())
.method(raw.method())
.version(raw.version())
// We always use an empty body because, in a Perseus request, only the URI matters
// Any custom data should therefore be sent in headers (if you're doing that, consider a dedicated API)
.body(())
Expand Down
6 changes: 3 additions & 3 deletions packages/perseus-actix-web/src/initial_load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use std::rc::Rc;
/// Builds on the internal Perseus primitives to provide a utility function that returns an `HttpResponse` automatically.
fn return_error_page(
url: &str,
status: &u16,
status: u16,
// This should already have been transformed into a string (with a source chain etc.)
err: &str,
translator: Option<Rc<Translator>>,
Expand All @@ -38,7 +38,7 @@ fn return_error_page(
html_shell,
root_id,
);
HttpResponse::build(StatusCode::from_u16(*status).unwrap())
HttpResponse::build(StatusCode::from_u16(status).unwrap())
.content_type("text/html")
.body(html)
}
Expand All @@ -62,7 +62,7 @@ pub async fn initial_load<M: MutableStore, T: TranslationsManager>(
let html_err = |status: u16, err: &str| {
return return_error_page(
path,
&status,
status,
err,
None,
error_pages,
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus-cli/src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ async fn core(dir: PathBuf) -> Result<i32, Error> {
}

// Parse the CLI options with `clap`
let opts: Opts = Opts::parse();
let opts = Opts::parse();
// Check the user's environment to make sure they have prerequisites
// We do this after any help pages or version numbers have been parsed for snappiness
check_env()?;
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus-cli/src/serve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ pub fn serve(dir: PathBuf, opts: ServeOpts) -> Result<(i32, Option<String>), Exe
Ok((exit_code, None))
} else {
// The user doesn't want to run the server, so we'll give them the executable path instead
let exec_str: String = (*exec.lock().unwrap()).to_string();
let exec_str = (*exec.lock().unwrap()).to_string();
println!("Not running server because `--no-run` was provided. You can run it manually by running the following executable in `.perseus/server/`.\n{}", &exec_str);
Ok((0, Some(exec_str)))
}
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus-macro/src/head.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl Parse for HeadFn {
};
// Can either accept a single argument for properties or no arguments
let mut inputs = sig.inputs.into_iter();
let arg: Option<FnArg> = inputs.next();
let arg = inputs.next();
// We don't care what the type is, as long as it's not `self`
if let Some(FnArg::Receiver(arg)) = arg {
return Err(syn::Error::new_spanned(
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus-macro/src/template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl Parse for TemplateFn {
};
// Can either accept a single argument for properties or no arguments
let mut inputs = sig.inputs.into_iter();
let arg: Option<FnArg> = inputs.next();
let arg = inputs.next();
// We don't care what the type is, as long as it's not `self`
if let Some(FnArg::Receiver(arg)) = arg {
return Err(syn::Error::new_spanned(arg, "templates can't take `self`"));
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus-macro/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl Parse for TestFn {
};
// Must accept a single argument for the Fantoccini client
let mut inputs = sig.inputs.into_iter();
let arg: FnArg = inputs.next().unwrap_or_else(|| syn::parse_quote! { _: () });
let arg = inputs.next().unwrap_or_else(|| syn::parse_quote! { _: () });
match &arg {
FnArg::Typed(_) => (),
// Can't accept `self`
Expand Down
6 changes: 3 additions & 3 deletions packages/perseus-warp/src/initial_load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use warp::{http::Response, path::FullPath};
/// Builds on the internal Perseus primitives to provide a utility function that returns a `Response` automatically.
fn return_error_page(
url: &str,
status: &u16,
status: u16,
// This should already have been transformed into a string (with a source chain etc.)
err: &str,
translator: Option<Rc<Translator>>,
Expand All @@ -36,7 +36,7 @@ fn return_error_page(
html_shell,
root_id,
);
Response::builder().status(*status).body(html).unwrap()
Response::builder().status(status).body(html).unwrap()
}

/// The handler for calls to any actual pages (first-time visits), which will render the appropriate HTML and then interpolate it into
Expand All @@ -60,7 +60,7 @@ pub async fn initial_load_handler<M: MutableStore, T: TranslationsManager>(
let html_err = |status: u16, err: &str| {
return return_error_page(
path,
&status,
status,
err,
None,
error_pages,
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ pub async fn build_templates_for_locale(
exporting: bool,
) -> Result<(), ServerError> {
// The render configuration stores a list of pages to the root paths of their templates
let mut render_cfg: HashMap<String, String> = HashMap::new();
let mut render_cfg = HashMap::new();
// Create each of the templates
let mut futs = Vec::new();
for template in templates.values() {
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/cache_res.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ where
&filename, err
)
});
let res: D = match serde_json::from_str(&contents) {
let res = match serde_json::from_str(&contents) {
Ok(cached_res) => cached_res,
// If the stuff in the cache can't be deserialized, we'll force a recreation (we don't recurse because that requires boxing the future)
Err(_) => {
Expand Down
22 changes: 11 additions & 11 deletions packages/perseus/src/error_pages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,41 +45,41 @@ impl<G: Html> ErrorPages<G> {
self.status_pages.insert(status, page);
}
/// Gets the internal template function to render.
fn get_template_fn(&self, status: &u16) -> &ErrorPageTemplate<G> {
fn get_template_fn(&self, status: u16) -> &ErrorPageTemplate<G> {
// Check if we have an explicitly defined page for this status code
// If not, we'll render the fallback page
match self.status_pages.contains_key(status) {
true => self.status_pages.get(status).unwrap(),
match self.status_pages.contains_key(&status) {
true => self.status_pages.get(&status).unwrap(),
false => &self.fallback,
}
}
/// Gets the template for a page without rendering it into a container.
pub fn get_template_for_page(
&self,
url: &str,
status: &u16,
status: u16,
err: &str,
translator: Option<Rc<Translator>>,
) -> View<G> {
let template_fn = self.get_template_fn(status);

template_fn(url.to_string(), *status, err.to_string(), translator)
template_fn(url.to_string(), status, err.to_string(), translator)
}
}
impl ErrorPages<DomNode> {
/// Renders the appropriate error page to the given DOM container.
pub fn render_page(
&self,
url: &str,
status: &u16,
status: u16,
err: &str,
translator: Option<Rc<Translator>>,
container: &Element,
) {
let template_fn = self.get_template_fn(status);
// Render that to the given container
sycamore::render_to(
|| template_fn(url.to_string(), *status, err.to_string(), translator),
|| template_fn(url.to_string(), status, err.to_string(), translator),
container,
);
}
Expand All @@ -90,15 +90,15 @@ impl ErrorPages<HydrateNode> {
pub fn hydrate_page(
&self,
url: &str,
status: &u16,
status: u16,
err: &str,
translator: Option<Rc<Translator>>,
container: &Element,
) {
let template_fn = self.get_template_fn(status);
// Render that to the given container
sycamore::hydrate_to(
|| template_fn(url.to_string(), *status, err.to_string(), translator),
|| template_fn(url.to_string(), status, err.to_string(), translator),
container,
);
}
Expand All @@ -108,14 +108,14 @@ impl ErrorPages<SsrNode> {
pub fn render_to_string(
&self,
url: &str,
status: &u16,
status: u16,
err: &str,
translator: Option<Rc<Translator>>,
) -> String {
let template_fn = self.get_template_fn(status);
// Render that to the given container
sycamore::render_to_string(|| {
template_fn(url.to_string(), *status, err.to_string(), translator)
template_fn(url.to_string(), status, err.to_string(), translator)
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/locales.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub struct Locales {
impl Locales {
/// Gets all the supported locales by combining the default, and other.
pub fn get_all(&self) -> Vec<&String> {
let mut vec: Vec<&String> = vec![&self.default];
let mut vec = vec![&self.default];
vec.extend(&self.other);

vec
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/plugins/functional.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ impl<A, R> PluginAction<A, R, HashMap<String, R>> for FunctionalPluginAction<A,
action_data: A,
plugin_data: &HashMap<String, Box<dyn Any + Send>>,
) -> HashMap<String, R> {
let mut returns: HashMap<String, R> = HashMap::new();
let mut returns = HashMap::new();
for (plugin_name, runner) in &self.runners {
let ret = runner(
&action_data,
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ pub fn match_route<G: Html>(
templates: &TemplateMap<G>,
locales: &Locales,
) -> RouteVerdict<G> {
let path_vec: Vec<&str> = path_slice.to_vec();
let path_vec = path_slice.to_vec();
let path_joined = path_vec.join("/"); // This should not have a leading forward slash, it's used for asset fetching by the app shell

let verdict;
Expand Down
4 changes: 2 additions & 2 deletions packages/perseus/src/server/build_error_page.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::rc::Rc;
/// exists then so the server doesn't have to do nearly as much work).
pub fn build_error_page(
url: &str,
status: &u16,
status: u16,
// This should already have been transformed into a string (with a source chain etc.)
err: &str,
translator: Option<Rc<Translator>>,
Expand All @@ -22,7 +22,7 @@ pub fn build_error_page(
// Right now, translators are never included in transmitted error pages
let error_page_data = ErrorPageData {
url: url.to_string(),
status: *status,
status,
err: err.to_string(),
};

Expand Down
13 changes: 6 additions & 7 deletions packages/perseus/src/server/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ pub async fn get_page_for_template(
// The same applies for the document metadata
let mut head = String::new();
// Multiple rendering strategies may need to amalgamate different states
let mut states: States = States::new();
let mut states = States::new();

// Handle build state (which might use revalidation or incremental)
if template.uses_build_state() || template.is_basic() {
Expand Down Expand Up @@ -374,14 +374,13 @@ pub async fn get_page_for_template(
// Otherwise we go as with HTML, request trumps build
// Of course, if only one state was defined, we'll just use that regardless (so `None` prioritization is impossible)
// If this is the case, the build content will still be served, and then it's up to the client to hydrate it with the new amalgamated state
let state: Option<String>;
if !states.both_defined() {
state = states.get_defined()?;
let state = if !states.both_defined() {
states.get_defined()?
} else if template.can_amalgamate_states() {
state = template.amalgamate_states(states)?;
template.amalgamate_states(states)?
} else {
state = states.request_state;
}
states.request_state
};

// Combine everything into one JSON object
let res = PageData {
Expand Down
Loading

0 comments on commit 2c0d344

Please sign in to comment.