Skip to content

Commit

Permalink
Append sitemap extension and optimize imafe metadata static generation (
Browse files Browse the repository at this point in the history 
#66477)

### What

Optimizing the static generation for dynamic metadata routes

If you're not using `generateSitemaps()` or `generateSitemaps()`, you
don't need to change any file conventions.
If you're using multi sitemap routes, make sure the returned `id`
properties from `generateSitemaps()` don't need to contain `.xml`, since
we'll always append one for you.

Analyzing the exports of metadata routes and determine if we need to
make them as dynamic routes.

### Why

Previously, users are struggling with the multi routes of sitemap or
images.
For sitemap, the `.xml` extension in url doesn't get appended
consistently to the multi sitemap route between dev and prod.
For image routes, the generated image routes are always dynamic routes
which cannot get static optimized.

The reason is that we need to always generate a catch-all route (such as
`/icon/[[...id]]` to handle both single route case (e.g. without
`generateImageMetadata`, representing url `/icon`) or multi route (e.g.
with `generateImageMetadata`, representing url `/icon/[id]`), only
catch-all routes can do it. This approach fail the static optimization
and make mapping url pretty difficult as parsing the file to check the
module exports has to be done before it.

#### Benifits

For image routes urls, this approach could help on static generation
such as single `/opengraph-image` route can be treated as static, and
then it can get static optimized if possible.

**Before**: `/opengraph-image/[[...id]]` cannot be optimized
**After**: single route `/opengraph-image` and multi-route
`/opengraph-image/[id]` are both possible to be statically optimized

For sitemap, since we removed appending `.xml` for dynamic routes, it’s
hard for users to have `/sitemap.xml` url with dynamic route convention
`sitemap.js` . But users desire smooth migration and flexibility.

**Before**: In v15 rc we removed the `.xml` appending that `sitemap.js`
will generate url `/sitemap` makes users hard to migrate, as users need
to re-submit the new sitemap url.
**After**: Now we'll consistently generate the `.xml`. Single route will
become `/sitemap.xml`, and multi route will become `/sitemap/[id].xml`.
It's still better than v15 as the urls generation is consistent, no
difference between dev and prod.

Here's the url generation comparsion

#### Before

All the routes are dynamic which cannot be optimized, we only had a
hacky optimization for prodution build multi-routes sitemap routes

| | only default export | `export generateImageMetadata()` | `export
generateSitemaps()` |
| -- | -- | -- | -- |
| opengraph-image.js | /opengraph-image/[[...id]] |
/opengraph-image[[...id]]/ | /opengraph-image/[[...id]] |
| sitemap.js | /sitemap/[[...id]] | /sitemap/[[...id]] | dev:
`/sitemap/[[...id]]` prod: `/sitemap/[id]` |

#### After

Most of the single route will are to get statically optimized now, and
the multi-routes sitemap are able to get SSG now

| | only default export | `export generateImageMetadata()` | `export
generateSitemaps()` |
| -- | -- | -- | -- |
| opengraph-image.js | /opengraph-image | /opengraph-image/[id] | - |
| sitemap.js | /sitemap.xml | - | /sitemap/[id].xml |

Next.js will have less overhead of mapping urls, we can easily multiply
the urls generation simply based on file conventions.

x-ref: feedback from #65507 
Closes #66232
  • Loading branch information
@huozhi
huozhi authored Jun 10, 2024
1 parent 544fc0a commit f893c18
Show file tree
Hide file tree
Showing 28 changed files with 563 additions and 295 deletions.
6 changes: 3 additions & 3 deletions docs/02-app/02-api-reference/02-file-conventions/01-metadata/sitemap.mdx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export default function sitemap() {

Output:

```xml filename="acme.com/sitemap"
```xml filename="acme.com/sitemap.xml"
<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
<url>
<loc>https://acme.com</loc>
Expand Down Expand Up @@ -163,7 +163,7 @@ export default function sitemap(): MetadataRoute.Sitemap {

Output:

```xml filename="acme.com/sitemap"
```xml filename="acme.com/sitemap.xml"
<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9" xmlns:xhtml="http://www.w3.org/1999/xhtml">
<url>
<loc>https://acme.com</loc>
Expand Down Expand Up @@ -264,7 +264,7 @@ export default async function sitemap({ id }) {
}
```

Your generated sitemaps will be available at `/.../sitemap/[id]`. For example, `/product/sitemap/1`.
Your generated sitemaps will be available at `/.../sitemap/[id]`. For example, `/product/sitemap/1.xml`.

See the [`generateSitemaps` API reference](/docs/app/api-reference/functions/generate-sitemaps) for more information.

Expand Down
44 changes: 35 additions & 9 deletions packages/next-swc/crates/next-core/src/app_segment_config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use turbo_tasks_fs::FileSystemPath;
use turbopack_binding::{
swc::core::{
common::{source_map::Pos, Span, Spanned, GLOBALS},
ecma::ast::{Expr, Ident, Program},
ecma::ast::{Decl, Expr, FnExpr, Ident, Program},
},
turbopack::{
core::{
Expand Down Expand Up @@ -73,6 +73,9 @@ pub struct NextSegmentConfig {
pub runtime: Option<NextRuntime>,
pub preferred_region: Option<Vec<RcStr>>,
pub experimental_ppr: Option<bool>,
/// Wether these metadata exports are defined in the source file.
pub generate_image_metadata: bool,
pub generate_sitemaps: bool,
}

#[turbo_tasks::value_impl]
Expand All @@ -95,6 +98,7 @@ impl NextSegmentConfig {
runtime,
preferred_region,
experimental_ppr,
..
} = self;
*dynamic = dynamic.or(parent.dynamic);
*dynamic_params = dynamic_params.or(parent.dynamic_params);
Expand Down Expand Up @@ -137,6 +141,7 @@ impl NextSegmentConfig {
runtime,
preferred_region,
experimental_ppr,
..
} = self;
merge_parallel(dynamic, &parallel_config.dynamic, "dynamic")?;
merge_parallel(
Expand Down Expand Up @@ -272,22 +277,35 @@ pub async fn parse_segment_config_from_source(
let mut config = NextSegmentConfig::default();

for item in &module_ast.body {
let Some(decl) = item
let Some(export_decl) = item
.as_module_decl()
.and_then(|mod_decl| mod_decl.as_export_decl())
.and_then(|export_decl| export_decl.decl.as_var())
else {
continue;
};

for decl in &decl.decls {
let Some(ident) = decl.name.as_ident().map(|ident| ident.deref()) else {
continue;
};
match &export_decl.decl {
Decl::Var(var_decl) => {
for decl in &var_decl.decls {
let Some(ident) = decl.name.as_ident().map(|ident| ident.deref()) else {
continue;
};

if let Some(init) = decl.init.as_ref() {
parse_config_value(source, &mut config, ident, init, eval_context);
if let Some(init) = decl.init.as_ref() {
parse_config_value(source, &mut config, ident, init, eval_context);
}
}
}
Decl::Fn(fn_decl) => {
let ident = &fn_decl.ident;
// create an empty expression of {}, we don't need init for function
let init = Expr::Fn(FnExpr {
ident: None,
function: fn_decl.function.clone(),
});
parse_config_value(source, &mut config, ident, &init, eval_context);
}
_ => {}
}
}
config
Expand Down Expand Up @@ -431,6 +449,14 @@ fn parse_config_value(

config.preferred_region = Some(preferred_region);
}
// Match exported generateImageMetadata function and generateSitemaps function, and pass
// them to config.
"generateImageMetadata" => {
config.generate_image_metadata = true;
}
"generateSitemaps" => {
config.generate_sitemaps = true;
}
"experimental_ppr" => {
let value = eval_context.eval(init);
let Some(val) = value.as_bool() else {
Expand Down
16 changes: 3 additions & 13 deletions packages/next-swc/crates/next-core/src/next_app/metadata/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,8 +306,7 @@ pub fn normalize_metadata_route(mut page: AppPage) -> Result<AppPage> {
route += ".txt"
} else if route == "/manifest" {
route += ".webmanifest"
// Do not append the suffix for the sitemap route
} else if !route.ends_with("/sitemap") {
} else {
// Remove the file extension, e.g. /route-path/robots.txt -> /route-path
let pathname_prefix = split_directory(&route).0.unwrap_or_default();
suffix = get_metadata_route_suffix(pathname_prefix);
Expand All @@ -317,13 +316,8 @@ pub fn normalize_metadata_route(mut page: AppPage) -> Result<AppPage> {
// /<metadata-route>/route.ts. If it's a metadata file route, we need to
// append /[id]/route to the page.
if !route.ends_with("/route") {
let is_static_metadata_file = is_static_metadata_route_file(&page.to_string());
let (base_name, ext) = split_extension(&route);

let is_static_route = route.starts_with("/robots")
|| route.starts_with("/manifest")
|| is_static_metadata_file;

page.0.pop();

page.push(PageSegment::Static(
Expand All @@ -338,10 +332,6 @@ pub fn normalize_metadata_route(mut page: AppPage) -> Result<AppPage> {
.into(),
))?;

if !is_static_route {
page.push(PageSegment::OptionalCatchAll("__metadata_id__".into()))?;
}

page.push(PageSegment::PageType(PageType::Route))?;
}

Expand All @@ -358,11 +348,11 @@ mod test {
let cases = vec![
[
"/client/(meme)/more-route/twitter-image",
"/client/(meme)/more-route/twitter-image-769mad/[[...__metadata_id__]]/route",
"/client/(meme)/more-route/twitter-image-769mad/route",
],
[
"/client/(meme)/more-route/twitter-image2",
"/client/(meme)/more-route/twitter-image2-769mad/[[...__metadata_id__]]/route",
"/client/(meme)/more-route/twitter-image2-769mad/route",
],
["/robots.txt", "/robots.txt/route"],
["/manifest.webmanifest", "/manifest.webmanifest/route"],
Expand Down
102 changes: 65 additions & 37 deletions packages/next-swc/crates/next-core/src/next_app/metadata/route.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//!
//! See `next/src/build/webpack/loaders/next-metadata-route-loader`
use anyhow::{bail, Result};
use anyhow::{bail, Ok, Result};
use base64::{display::Base64Display, engine::general_purpose::STANDARD};
use indoc::{formatdoc, indoc};
use turbo_tasks::{ValueToString, Vc};
Expand All @@ -22,17 +22,19 @@ use super::get_content_type;
use crate::{
app_structure::MetadataItem,
mode::NextMode,
next_app::{app_entry::AppEntry, app_route_entry::get_app_route_entry, AppPage, PageSegment},
next_app::{
app_entry::AppEntry, app_route_entry::get_app_route_entry, AppPage, PageSegment, PageType,
},
next_config::NextConfig,
parse_segment_config_from_source,
};

/// Computes the route source for a Next.js metadata file.
#[turbo_tasks::function]
pub async fn get_app_metadata_route_source(
page: AppPage,
mode: NextMode,
metadata: MetadataItem,
is_multi_dynamic: bool,
) -> Result<Vc<Box<dyn Source>>> {
Ok(match metadata {
MetadataItem::Static { path } => static_route_source(mode, path),
Expand All @@ -43,7 +45,7 @@ pub async fn get_app_metadata_route_source(
if stem == "robots" || stem == "manifest" {
dynamic_text_route_source(path)
} else if stem == "sitemap" {
dynamic_site_map_route_source(mode, path, page)
dynamic_site_map_route_source(mode, path, is_multi_dynamic)
} else {
dynamic_image_route_source(path)
}
Expand All @@ -52,11 +54,11 @@ pub async fn get_app_metadata_route_source(
}

#[turbo_tasks::function]
pub fn get_app_metadata_route_entry(
pub async fn get_app_metadata_route_entry(
nodejs_context: Vc<ModuleAssetContext>,
edge_context: Vc<ModuleAssetContext>,
project_root: Vc<FileSystemPath>,
page: AppPage,
mut page: AppPage,
mode: NextMode,
metadata: MetadataItem,
next_config: Vc<NextConfig>,
Expand All @@ -69,11 +71,43 @@ pub fn get_app_metadata_route_entry(

let source = Vc::upcast(FileSource::new(original_path));
let segment_config = parse_segment_config_from_source(source);
let is_dynamic_metadata = matches!(metadata, MetadataItem::Dynamic { .. });
let is_multi_dynamic: bool = if Some(segment_config).is_some() {
// is_multi_dynamic is true when config.generateSitemaps or
// config.generateImageMetadata is defined in dynamic routes
let config = segment_config.await.unwrap();
config.generate_sitemaps || config.generate_image_metadata
} else {
false
};

// Map dynamic sitemap and image routes based on the exports.
// if there's generator export: add /[__metadata_id__] to the route;
// otherwise keep the original route.
// For sitemap, if the last segment is sitemap, appending .xml suffix.
if is_dynamic_metadata {
// remove the last /route segment of page
page.0.pop();

let _ = if is_multi_dynamic {
page.push(PageSegment::Dynamic("__metadata_id__".into()))
} else {
// if page last segment is sitemap, change to sitemap.xml
if page.last() == Some(&PageSegment::Static("sitemap".into())) {
page.0.pop();
page.push(PageSegment::Static("sitemap.xml".into()))
} else {
Ok(())
}
};
// Push /route back
let _ = page.push(PageSegment::PageType(PageType::Route));
};

get_app_route_entry(
nodejs_context,
edge_context,
get_app_metadata_route_source(page.clone(), mode, metadata),
get_app_metadata_route_source(mode, metadata, is_multi_dynamic),
page,
project_root,
Some(segment_config),
Expand Down Expand Up @@ -208,26 +242,24 @@ async fn dynamic_text_route_source(path: Vc<FileSystemPath>) -> Result<Vc<Box<dy
async fn dynamic_site_map_route_source(
mode: NextMode,
path: Vc<FileSystemPath>,
page: AppPage,
is_multi_dynamic: bool,
) -> Result<Vc<Box<dyn Source>>> {
let stem = path.file_stem().await?;
let stem = stem.as_deref().unwrap_or_default();
let ext = &*path.extension().await?;

let content_type = get_content_type(path).await?;

let mut static_generation_code = "";

if mode.is_production() && page.contains(&PageSegment::Dynamic("[__metadata_id__]".into())) {
if mode.is_production() && is_multi_dynamic {
static_generation_code = indoc! {
r#"
export async function generateStaticParams() {
const sitemaps = await generateSitemaps()
const params = []
for (const item of sitemaps) {
params.push({ __metadata_id__: item.id.toString() })
}
for (const item of sitemaps) {{
params.push({ __metadata_id__: item.id.toString() + '.xml' })
}}
return params
}
"#,
Expand All @@ -252,29 +284,25 @@ async fn dynamic_site_map_route_source(
}}
export async function GET(_, ctx) {{
const {{ __metadata_id__ = [], ...params }} = ctx.params || {{}}
const targetId = __metadata_id__[0]
let id = undefined
const sitemaps = generateSitemaps ? await generateSitemaps() : null
const {{ __metadata_id__: id, ...params }} = ctx.params || {{}}
const hasXmlExtension = id ? id.endsWith('.xml') : false
if (id && !hasXmlExtension) {{
return new NextResponse('Not Found', {{
status: 404,
}})
}}
if (sitemaps) {{
id = sitemaps.find((item) => {{
if (process.env.NODE_ENV !== 'production') {{
if (item?.id == null) {{
throw new Error('id property is required for every item returned from generateSitemaps')
}}
if (process.env.NODE_ENV !== 'production' && sitemapModule.generateSitemaps) {{
const sitemaps = await sitemapModule.generateSitemaps()
for (const item of sitemaps) {{
if (item?.id == null) {{
throw new Error('id property is required for every item returned from generateSitemaps')
}}
return item.id.toString() === targetId
}})?.id
if (id == null) {{
return new NextResponse('Not Found', {{
status: 404,
}})
}}
}}
const data = await handler({{ id }})
const targetId = id && hasXmlExtension ? id.slice(0, -4) : undefined
const data = await handler({{ id: targetId }})
const content = resolveRouteData(data, fileType)
return new NextResponse(content, {{
Expand Down Expand Up @@ -324,12 +352,12 @@ async fn dynamic_image_route_source(path: Vc<FileSystemPath>) -> Result<Vc<Box<d
}}
export async function GET(_, ctx) {{
const {{ __metadata_id__ = [], ...params }} = ctx.params || {{}}
const targetId = __metadata_id__[0]
const {{ __metadata_id__, ...params }} = ctx.params || {{}}
const targetId = __metadata_id__
let id = undefined
const imageMetadata = generateImageMetadata ? await generateImageMetadata({{ params }}) : null
if (imageMetadata) {{
if (generateImageMetadata) {{
const imageMetadata = await generateImageMetadata({{ params }})
id = imageMetadata.find((item) => {{
if (process.env.NODE_ENV !== 'production') {{
if (item?.id == null) {{
Expand Down
Loading

0 comments on commit f893c18

Please sign in to comment.