From 197956bc734bc1d85f56bcfc7b327bb1ed1f4c07 Mon Sep 17 00:00:00 2001 From: arctic_hen7 Date: Sun, 19 Sep 2021 11:04:15 +1000 Subject: [PATCH] =?UTF-8?q?fix(routing):=20=F0=9F=90=9B=20fixed=20link=20h?= =?UTF-8?q?andling=20errors=20in=20#8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Links now behave correctly, but are still subject to #8. --- examples/cli/.perseus/Cargo.toml | 1 + examples/cli/.perseus/src/lib.rs | 87 +++++++----- packages/perseus/src/shell.rs | 233 ++++++++++++++++++------------- 3 files changed, 184 insertions(+), 137 deletions(-) diff --git a/examples/cli/.perseus/Cargo.toml b/examples/cli/.perseus/Cargo.toml index 48a85620d2..b2efea9bba 100644 --- a/examples/cli/.perseus/Cargo.toml +++ b/examples/cli/.perseus/Cargo.toml @@ -17,6 +17,7 @@ sycamore = { version = "0.6", features = ["ssr"] } sycamore-router = "0.6" web-sys = { version = "0.3", features = ["Headers", "Request", "RequestInit", "RequestMode", "Response", "ReadableStream", "Window"] } wasm-bindgen = { version = "0.2", features = ["serde-serialize"] } +wasm-bindgen-futures = "0.4" serde = { version = "1", features = ["derive"] } serde_json = "1" # Possibly don't need? console_error_panic_hook = "0.1.6" diff --git a/examples/cli/.perseus/src/lib.rs b/examples/cli/.perseus/src/lib.rs index 37fe4cff31..702ba014a1 100644 --- a/examples/cli/.perseus/src/lib.rs +++ b/examples/cli/.perseus/src/lib.rs @@ -5,7 +5,7 @@ use perseus::shell::{get_initial_state, get_render_cfg, InitialState}; use perseus::{app_shell, create_app_route, detect_locale, ClientTranslationsManager, DomNode}; use std::cell::RefCell; use std::rc::Rc; -use sycamore::prelude::{template, StateHandle}; +use sycamore::prelude::{cloned, template, NodeRef, StateHandle}; use sycamore_router::{HistoryIntegration, Router, RouterProps}; use wasm_bindgen::{prelude::wasm_bindgen, JsValue}; @@ -23,14 +23,17 @@ pub fn run() -> Result<(), JsValue> { .unwrap() .unwrap(); - // Get the root we'll be injecting actual content into (created by the server) + // Get the root that the server will have injected initial load content into + // This will be moved into a reactive `
` by the app shell // This is an `Option` until we know we aren't doing loclae detection (in which case it wouldn't exist) - let container = web_sys::window() + let initial_container = web_sys::window() .unwrap() .document() .unwrap() .query_selector("#__perseus_content") .unwrap(); + // And create a node reference that we can use as a handle to the reactive verison + let container_rx = NodeRef::new(); // Create a mutable translations manager to control caching let translations_manager = @@ -51,41 +54,49 @@ pub fn run() -> Result<(), JsValue> { || { template! { Router(RouterProps::new(HistoryIntegration::new(), move |route: StateHandle>| { - match &route.get().as_ref().0 { - // Perseus' custom routing system is tightly coupled to the template system, and returns exactly what we need for the app shell! - // If a non-404 error occurred, it will be handled in the app shell - RouteVerdict::Found(RouteInfo { - path, - template, - locale - }) => app_shell( - path.clone(), - template.clone(), - locale.clone(), - // We give the app shell a translations manager and let it get the `Rc` itself (because it can do async safely) - Rc::clone(&translations_manager), - Rc::clone(&error_pages), - container.unwrap().clone() - ), - // If the user is using i18n, then they'll want to detect the locale on any paths missing a locale - // Those all go to the same system that redirects to the appropriate locale - // Note that `container` doesn't exist for this scenario - RouteVerdict::LocaleDetection(path) => detect_locale(path.clone(), get_locales()), - // We handle the 404 for the user for convenience - // To get a translator here, we'd have to go async and dangerously check the URL - // If this is an initial load, there'll already be an error message, so we should only proceed if the declaration is not `error` - RouteVerdict::NotFound => { - if let InitialState::Error(ErrorPageData { url, status, err }) = get_initial_state() { - // Hydrate the error pages - // Right now, we don't provide translators to any error pages that have come from the server - error_pages.hydrate_page(&url, &status, &err, None, &container.unwrap()); - } else { - get_error_pages::().get_template_for_page("", &404, "not found", None); - } - }, - }; - // Everything is based on hydration, and so we always return an empty template - sycamore::template::Template::empty() + wasm_bindgen_futures::spawn_local(cloned!((container_rx) => async move { + let container_rx_elem = container_rx.get::().unchecked_into::(); + match &route.get().as_ref().0 { + // Perseus' custom routing system is tightly coupled to the template system, and returns exactly what we need for the app shell! + // If a non-404 error occurred, it will be handled in the app shell + RouteVerdict::Found(RouteInfo { + path, + template, + locale + }) => app_shell( + path.clone(), + template.clone(), + locale.clone(), + // We give the app shell a translations manager and let it get the `Rc` itself (because it can do async safely) + Rc::clone(&translations_manager), + Rc::clone(&error_pages), + initial_container.unwrap().clone(), + container_rx_elem.clone() + ).await, + // If the user is using i18n, then they'll want to detect the locale on any paths missing a locale + // Those all go to the same system that redirects to the appropriate locale + // Note that `container` doesn't exist for this scenario + RouteVerdict::LocaleDetection(path) => detect_locale(path.clone(), get_locales()), + // We handle the 404 for the user for convenience + // To get a translator here, we'd have to go async and dangerously check the URL + // If this is an initial load, there'll already be an error message, so we should only proceed if the declaration is not `error` + RouteVerdict::NotFound => { + if let InitialState::Error(ErrorPageData { url, status, err }) = get_initial_state() { + // Hydrate the error pages + // Right now, we don't provide translators to any error pages that have come from the server + error_pages.hydrate_page(&url, &status, &err, None, &container_rx_elem); + } else { + get_error_pages::().get_template_for_page("", &404, "not found", None); + } + }, + }; + })); + // This template is reactive, and will be updated as necessary + // However, the server has already rendered initial load content elsewhere, so we move that into here as well in the app shell + // The main reason for this is that the router only intercepts click events from its children + template! { + div(class="__perseus_content_rx", ref=container_rx) {} + } })) } }, diff --git a/packages/perseus/src/shell.rs b/packages/perseus/src/shell.rs index b65b2d034c..d75753dc95 100644 --- a/packages/perseus/src/shell.rs +++ b/packages/perseus/src/shell.rs @@ -134,13 +134,14 @@ extern "C" { /// Fetches the information for the given page and renders it. This should be provided the actual path of the page to render (not just the /// broader template). Asynchronous Wasm is handled here, because only a few cases need it. // TODO handle exceptions higher up -pub fn app_shell( +pub async fn app_shell( path: String, template: Template, locale: String, translations_manager: Rc>, error_pages: Rc>, - container: Element, + initial_container: Element, // The container that the server put initial load content into + container_rx_elem: Element, // The container that we'll actually use (reactive) ) { // Check if this was an initial load and we already have the state let initial_state = get_initial_state(); @@ -150,117 +151,151 @@ pub fn app_shell( InitialState::Present(state) => { // Unset the initial state variable so we perform subsequent renders correctly unset_initial_state(); - wasm_bindgen_futures::spawn_local(cloned!((container) => async move { - // Now that the user can see something, we can get the translator - let mut translations_manager_mut = translations_manager.borrow_mut(); - // This gets an `Rc` that references the translations manager, meaning no cloning of translations - let translator = translations_manager_mut.get_translator_for_locale(&locale).await; - let translator = match translator { - Ok(translator) => translator, - Err(err) => { - // Directly eliminate the HTMl sent in from the server before we render an error page - container.set_inner_html(""); - match err.kind() { - // These errors happen because we couldn't get a translator, so they certainly don't get one - ErrorKind::AssetNotOk(url, status, _) => return error_pages.render_page(url, status, &err.to_string(), None, &container), - ErrorKind::AssetSerFailed(url, _) => return error_pages.render_page(url, &500, &err.to_string(), None, &container), - ErrorKind::LocaleNotSupported(locale) => return error_pages.render_page(&format!("/{}/...", locale), &404, &err.to_string(),None, &container), - // No other errors should be returned - _ => panic!("expected 'AssetNotOk'/'AssetSerFailed'/'LocaleNotSupported' error, found other unacceptable error") - } + // We need to move the server-rendered content from its current container to the reactive container (otherwise Sycamore can't work with it properly) + let initial_html = initial_container.inner_html(); + container_rx_elem.set_inner_html(&initial_html); + initial_container.set_inner_html(""); + // Now that the user can see something, we can get the translator + let mut translations_manager_mut = translations_manager.borrow_mut(); + // This gets an `Rc` that references the translations manager, meaning no cloning of translations + let translator = translations_manager_mut + .get_translator_for_locale(&locale) + .await; + let translator = match translator { + Ok(translator) => translator, + Err(err) => { + // Directly eliminate the HTMl sent in from the server before we render an error page + container_rx_elem.set_inner_html(""); + match err.kind() { + // These errors happen because we couldn't get a translator, so they certainly don't get one + ErrorKind::AssetNotOk(url, status, _) => return error_pages.render_page(url, status, &err.to_string(), None, &container_rx_elem), + ErrorKind::AssetSerFailed(url, _) => return error_pages.render_page(url, &500, &err.to_string(), None, &container_rx_elem), + ErrorKind::LocaleNotSupported(locale) => return error_pages.render_page(&format!("/{}/...", locale), &404, &err.to_string(),None, &container_rx_elem), + // No other errors should be returned + _ => panic!("expected 'AssetNotOk'/'AssetSerFailed'/'LocaleNotSupported' error, found other unacceptable error") } - }; + } + }; - // Hydrate that static code using the acquired state - // BUG (Sycamore): this will double-render if the component is just text (no nodes) - sycamore::hydrate_to( - // This function provides translator context as needed - || template.render_for_template(state, Rc::clone(&translator)), - &container - ); - })); + // Hydrate that static code using the acquired state + // BUG (Sycamore): this will double-render if the component is just text (no nodes) + sycamore::hydrate_to( + // This function provides translator context as needed + || template.render_for_template(state, Rc::clone(&translator)), + &container_rx_elem, + ); } // If we have no initial state, we should proceed as usual, fetching the content and state from the server InitialState::NotPresent => { - // Spawn a Rust futures thread in the background to fetch the static HTML/JSON - wasm_bindgen_futures::spawn_local(cloned!((container) => async move { - // Get the static page data - let asset_url = format!("/.perseus/page/{}/{}?template_name={}", locale, path.to_string(), template.get_path()); - // If this doesn't exist, then it's a 404 (we went here by explicit navigation, but it may be an unservable ISR page or the like) - let page_data_str = fetch(&asset_url).await; - match page_data_str { - Ok(page_data_str) => match page_data_str { - Some(page_data_str) => { - // All good, deserialize the page data - let page_data = serde_json::from_str::(&page_data_str); - match page_data { - Ok(page_data) => { - // We have the page data ready, render everything - // Interpolate the HTML directly into the document (we'll hydrate it later) - container.set_inner_html(&page_data.content); + // Get the static page data + let asset_url = format!( + "/.perseus/page/{}/{}?template_name={}", + locale, + path.to_string(), + template.get_path() + ); + // If this doesn't exist, then it's a 404 (we went here by explicit navigation, but it may be an unservable ISR page or the like) + let page_data_str = fetch(&asset_url).await; + match page_data_str { + Ok(page_data_str) => match page_data_str { + Some(page_data_str) => { + // All good, deserialize the page data + let page_data = serde_json::from_str::(&page_data_str); + match page_data { + Ok(page_data) => { + // We have the page data ready, render everything + // Interpolate the HTML directly into the document (we'll hydrate it later) + container_rx_elem.set_inner_html(&page_data.content); - // Now that the user can see something, we can get the translator - let mut translations_manager_mut = translations_manager.borrow_mut(); - // This gets an `Rc` that references the translations manager, meaning no cloning of translations - let translator = translations_manager_mut.get_translator_for_locale(&locale).await; - let translator = match translator { - Ok(translator) => translator, - Err(err) => match err.kind() { - // These errors happen because we couldn't get a translator, so they certainly don't get one - ErrorKind::AssetNotOk(url, status, _) => return error_pages.render_page(url, status, &err.to_string(), None, &container), - ErrorKind::AssetSerFailed(url, _) => return error_pages.render_page(url, &500, &err.to_string(), None, &container), - ErrorKind::LocaleNotSupported(locale) => return error_pages.render_page(&format!("/{}/...", locale), &404, &err.to_string(),None, &container), - // No other errors should be returned - _ => panic!("expected 'AssetNotOk'/'AssetSerFailed'/'LocaleNotSupported' error, found other unacceptable error") - } - }; + // Now that the user can see something, we can get the translator + let mut translations_manager_mut = + translations_manager.borrow_mut(); + // This gets an `Rc` that references the translations manager, meaning no cloning of translations + let translator = translations_manager_mut + .get_translator_for_locale(&locale) + .await; + let translator = match translator { + Ok(translator) => translator, + Err(err) => match err.kind() { + // These errors happen because we couldn't get a translator, so they certainly don't get one + ErrorKind::AssetNotOk(url, status, _) => return error_pages.render_page(url, status, &err.to_string(), None, &container_rx_elem), + ErrorKind::AssetSerFailed(url, _) => return error_pages.render_page(url, &500, &err.to_string(), None, &container_rx_elem), + ErrorKind::LocaleNotSupported(locale) => return error_pages.render_page(&format!("/{}/...", locale), &404, &err.to_string(),None, &container_rx_elem), + // No other errors should be returned + _ => panic!("expected 'AssetNotOk'/'AssetSerFailed'/'LocaleNotSupported' error, found other unacceptable error") + } + }; - // Render the document head - let head_str = template.render_head_str(page_data.state.clone(), Rc::clone(&translator)); - // Get the current head - let head_elem = web_sys::window() - .unwrap() - .document() - .unwrap() - .query_selector("head") - .unwrap() - .unwrap(); - let head_html = head_elem.inner_html(); - // We'll assume that there's already previously interpolated head in addition to the hardcoded stuff, but it will be separated by the server-injected delimiter comment - // Thus, we replace the stuff after that delimiter comment with the new head - let head_parts: Vec<&str> = head_html.split("").collect(); - let new_head = format!("{}\n\n{}", head_parts[0], head_str); - head_elem.set_inner_html(&new_head); + // Render the document head + let head_str = template.render_head_str( + page_data.state.clone(), + Rc::clone(&translator), + ); + // Get the current head + let head_elem = web_sys::window() + .unwrap() + .document() + .unwrap() + .query_selector("head") + .unwrap() + .unwrap(); + let head_html = head_elem.inner_html(); + // We'll assume that there's already previously interpolated head in addition to the hardcoded stuff, but it will be separated by the server-injected delimiter comment + // Thus, we replace the stuff after that delimiter comment with the new head + let head_parts: Vec<&str> = head_html + .split("") + .collect(); + let new_head = format!( + "{}\n\n{}", + head_parts[0], head_str + ); + head_elem.set_inner_html(&new_head); - // Hydrate that static code using the acquired state - // BUG (Sycamore): this will double-render if the component is just text (no nodes) - sycamore::hydrate_to( - // This function provides translator context as needed - || template.render_for_template(page_data.state, Rc::clone(&translator)), - &container - ); - }, - // If the page failed to serialize, an exception has occurred - Err(err) => panic!("page data couldn't be serialized: '{}'", err) - }; - }, - // No translators ready yet - None => error_pages.render_page(&asset_url, &404, "page not found", None, &container), - }, - Err(err) => match err.kind() { - // No translators ready yet - ErrorKind::AssetNotOk(url, status, _) => error_pages.render_page(url, status, &err.to_string(), None, &container), - // No other errors should be returned - _ => panic!("expected 'AssetNotOk' error, found other unacceptable error") + // Hydrate that static code using the acquired state + // BUG (Sycamore): this will double-render if the component is just text (no nodes) + sycamore::hydrate_to( + // This function provides translator context as needed + || { + template.render_for_template( + page_data.state, + Rc::clone(&translator), + ) + }, + &container_rx_elem, + ); + } + // If the page failed to serialize, an exception has occurred + Err(err) => panic!("page data couldn't be serialized: '{}'", err), + }; } - }; - })); + // No translators ready yet + None => error_pages.render_page( + &asset_url, + &404, + "page not found", + None, + &container_rx_elem, + ), + }, + Err(err) => match err.kind() { + // No translators ready yet + ErrorKind::AssetNotOk(url, status, _) => error_pages.render_page( + url, + status, + &err.to_string(), + None, + &container_rx_elem, + ), + // No other errors should be returned + _ => panic!("expected 'AssetNotOk' error, found other unacceptable error"), + }, + }; } // Nothing should be done if an error was sent down InitialState::Error(ErrorPageData { url, status, err }) => { // Hydrate the currently static error page // Right now, we don't provide translators to any error pages that have come from the server - error_pages.hydrate_page(&url, &status, &err, None, &container); + error_pages.hydrate_page(&url, &status, &err, None, &container_rx_elem); } }; }