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

Incremental generation is broken for a template named index in the beta.19 #262

Closed
Miroito opened this issue Feb 26, 2023 · 4 comments
Closed
Labels
author-willing-to-impl The author of this issue is willing to try to implement the solution themselves. C-bug Category: bug tribble-reported This issue was reported through Tribble.

Comments

@Miroito
Copy link
Contributor

Miroito commented Feb 26, 2023

This issue is reporting a bug in the code of Perseus. Details of the scope will be available in issue labels.
The author described their issue as follows:

Incremental generation is broken for a template named index.

The steps to reproduce this issue are as follows:

See example template below.

A minimum reproducible example is available at <>.

  • Hydration-related: false
  • The author is willing to attempt a fix: true
Tribble internal data

dHJpYmJsZS1yZXBvcnRlZCxDLWJ1ZyxhdXRob3Itd2lsbGluZy10by1pbXBs

@github-actions github-actions bot added author-willing-to-impl The author of this issue is willing to try to implement the solution themselves. C-bug Category: bug tribble-reported This issue was reported through Tribble. labels Feb 26, 2023
@Miroito
Copy link
Contributor Author

Miroito commented Feb 26, 2023

It's not minimum but it gets the point across. It is a slightly modified version of the incremental generation example from the next docs.
Changing the template name to test (or anything else that is not index) will work properly.
In 0.3.6, incremental generation on index matched routes like /index/foo of index/foo/bar but not /foo (which is more than acceptable given using incremental generation on the index page is probably bad design in most cases)

use perseus::prelude::*;
use serde::{Deserialize, Serialize};
use sycamore::prelude::*;

#[derive(Serialize, Deserialize, Clone, ReactiveState)]
#[rx(alias = "PageStateRx")]
struct PageState {
    title: String,
    content: String,
}

#[auto_scope]
fn index_page<G: Html>(cx: Scope, state: &PageStateRx) -> View<G> {
    view! { cx,
    // Don't worry, there are much better ways of styling in Perseus!
      p {
          (state.title) (state.content)
      }
    }
}

#[engine_only_fn]
fn head(cx: Scope) -> View<SsrNode> {
    view! { cx,
        title { "Welcome to Perseus!" }
    }
}

pub fn get_template<G: Html>() -> Template<G> {
    Template::build("index")
        .build_paths_fn(get_build_paths)
        .build_state_fn(get_build_state)
        .view_with_state(index_page)
        .head(head)
        .incremental_generation()
        .build()
}


#[engine_only_fn]
async fn get_build_state(
    StateGeneratorInfo { path, .. }: StateGeneratorInfo<()>,
) -> Result<PageState, BlamedError<std::io::Error>> {
    // This path is illegal, and can't be rendered
    // Because we're using incremental generation, we could get literally anything
    // as the `path`
    let title = path.clone();
    let content = format!(
        "This is a post entitled '{}'. Its original slug was '{}'.",
        &title, &path
    );

    Ok(PageState { title, content })
}


#[engine_only_fn]
async fn get_build_paths() -> BuildPaths {
    BuildPaths {
        paths: vec!["".to_string()],
        extra: ().into(),
    }
}

@Miroito Miroito changed the title Incremental generation is broken for a template named index. Incremental generation is broken for a template named index in the beta.19 Feb 26, 2023
@arctic-hen7
Copy link
Member

From some initial testing, it seems like build paths still work perfectly, but incremental generation only works with / itself, failing in all other cases with a 404. The render configuration is generating /* as the key for this, which should be correct, as changing it to * manually fails a number of other pages. This looks like a forward slash stripping mistake somewhere, and I'll need to do some further investigation to figure out exactly what's causing this.

Chances are, this will be in the routing algorithm, since incremental generation is occurring flawlessly for the / route, and there shouldn't be any other barriers to it.

@arctic-hen7
Copy link
Member

Okay, this was a simple error on my part: the routing algorithm searches forward by splitting the path into segments over /, adding /* to the end of each progressive concatenation to see if there's an incremental match. This would only work for index-level incremental generation, however, if the first element of every path was empty, which is only true for the / route. Solved by adding an additional case to the end of the algorithm to match all routes when there's incremental generation at the index template (while preserving normal specificity requirements, i.e. more specific routes take precedence).

@arctic-hen7
Copy link
Member

This will be in beta 21.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author-willing-to-impl The author of this issue is willing to try to implement the solution themselves. C-bug Category: bug tribble-reported This issue was reported through Tribble.
Projects
None yet
Development

No branches or pull requests

2 participants