Skip to content

match next.js client side routing (buildManifest rewrites & devPagesManifest HMR) #3701

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

Merged
merged 8 commits into from
Feb 9, 2023
Merged

match next.js client side routing (buildManifest rewrites & devPagesManifest HMR) #3701

Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
2b562cf
add rewrites to buildManifest & serve it separately
ForsakenHarmony Feb 8, 2023
5dcd243
add HMR for `_devPagesManifest`
ForsakenHarmony Feb 8, 2023
aa7fd5e
don't reuse config loading in the router
ForsakenHarmony Feb 9, 2023
0177e72
utf8 content type for js
ForsakenHarmony Feb 9, 2023
3d9321b
clippy
ForsakenHarmony Feb 9, 2023
e9294f2
gotta actually test things sometimes
ForsakenHarmony Feb 9, 2023
302eda9
sort pages properly
ForsakenHarmony Feb 9, 2023
3116321
clippy my worst enemy >:(
ForsakenHarmony Feb 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 34 additions & 6 deletions crates/next-core/src/manifest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use anyhow::{Context, Result};
use indexmap::IndexMap;
use mime::{APPLICATION_JAVASCRIPT, APPLICATION_JSON};
use mime::{APPLICATION_JAVASCRIPT_UTF_8, APPLICATION_JSON};
use serde::Serialize;
use turbo_tasks::{
primitives::{StringVc, StringsVc},
Expand Down Expand Up @@ -74,7 +74,11 @@ impl DevManifestContentSourceVc {
.flatten()
.collect::<Vec<_>>();

routes.sort();
routes.sort_by_cached_key(|s| {
s.split("/")
.map(|e| PageSortKey::from(e))
.collect::<Vec<_>>()
});

Ok(StringsVc::cell(routes))
}
Expand Down Expand Up @@ -115,12 +119,12 @@ impl DevManifestContentSourceVc {
.collect();

let manifest = BuildManifest {
__rewrites: this.next_config.rewrites().await?,
rewrites: this.next_config.rewrites().await?,
sorted_pages,
routes,
};

let manifest = next_js_file("entry/buildManifest.js")
let manifest = next_js_file("entry/manifest/buildManifest.js")
.await?
.as_content()
.context("embedded buildManifest file missing")?
Expand All @@ -135,7 +139,8 @@ impl DevManifestContentSourceVc {
#[derive(Serialize)]
#[serde(rename_all = "camelCase")]
struct BuildManifest<'a> {
__rewrites: RewritesReadRef,
#[serde(rename = "__rewrites")]
rewrites: RewritesReadRef,
sorted_pages: &'a Vec<String>,

#[serde(flatten)]
Expand All @@ -162,7 +167,7 @@ impl ContentSource for DevManifestContentSource {
"_next/static/development/_buildManifest.js" => {
let build_manifest = &*self_vc.create_build_manifest().await?;

File::from(build_manifest.as_str()).with_content_type(APPLICATION_JAVASCRIPT)
File::from(build_manifest.as_str()).with_content_type(APPLICATION_JAVASCRIPT_UTF_8)
}
"_next/static/development/_devMiddlewareManifest.json" => {
// empty middleware manifest
Expand All @@ -177,3 +182,26 @@ impl ContentSource for DevManifestContentSource {
))
}
}

/// PageSortKey is necessary because the next.js client code looks for matches
/// in the order the pages are sent in the manifest,if they're sorted
/// alphabetically this means \[slug] and \[\[catchall]] routes are prioritized
/// over fixed paths, so we have to override the ordering with this.
#[derive(Ord, PartialOrd, Eq, PartialEq)]
enum PageSortKey {
Static(String),
Slug,
CatchAll,
}

impl From<&str> for PageSortKey {
Copy link
Contributor

@jridgewell jridgewell Feb 9, 2023

Choose a reason for hiding this comment

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

I think we can bound the lifetime of PageSortKey by the &str, so we don't need to clone into a String?

Suggested change
impl From<&str> for PageSortKey {
impl<'a> From<&'a str> for PageSortKey<'a> {

Copy link
Contributor Author

@ForsakenHarmony ForsakenHarmony Feb 9, 2023

Choose a reason for hiding this comment

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

I was assuming the &str wouldn't survive after returning out of the sort_by_cached_key lambda function, but let me check

Copy link
Contributor Author

@ForsakenHarmony ForsakenHarmony Feb 9, 2023

Choose a reason for hiding this comment

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

yeah

error: lifetime may not live long enough
  --> crates\next-core\src\manifest.rs:77:39
   |
77 |         routes.sort_by_cached_key(|s| s.split('/').map(PageSortKey::from).collect::<Vec<_>>());
   |                                    -- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2`
   |                                    ||
   |                                    |return type of closure is Vec<PageSortKey<'2>>
   |                                    has type `&'1 std::string::String`

fn from(value: &str) -> Self {
if value.starts_with("[[") && value.ends_with("]]") {
PageSortKey::CatchAll
} else if value.starts_with("[") && value.ends_with("]") {
PageSortKey::Slug
} else {
PageSortKey::Static(value.to_string())
}
}
}