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.

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

Jump to bottom

[Turbopack] fix rsc chunking optimization #70461

Merged
merged 6 commits into from
Sep 25, 2024
Merged

[Turbopack] fix rsc chunking optimization #70461

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f4b7e77
fix query string for rsc entry
sokra Sep 25, 2024
ea7ce0e
mark all modules as server components correctly, fix order of imports…
sokra Sep 25, 2024
58560e8
find server utils and put them into a separate chunk group
sokra Sep 25, 2024
12b99e1
error message spacing
sokra Sep 25, 2024
de6f369
fix test case
sokra Sep 25, 2024
1a75edb
Update crates/next-core/src/next_app/app_page_entry.rs
sokra Sep 25, 2024
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
67 changes: 53 additions & 14 deletions crates/next-api/src/app.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ use next_core::{
next_app::{
app_client_references_chunks::get_app_server_reference_modules,
get_app_client_references_chunks, get_app_client_shared_chunk_group, get_app_page_entry,
get_app_route_entry, metadata::route::get_app_metadata_route_entry, AppEntry, AppPage,
get_app_route_entry, include_modules_module::IncludeModulesModule,
metadata::route::get_app_metadata_route_entry, AppEntry, AppPage,
},
next_client::{
get_client_module_options_context, get_client_resolve_options_context,
Expand All @@ -36,7 +37,9 @@ use next_core::{
};
use serde::{Deserialize, Serialize};
use tracing::Instrument;
use turbo_tasks::{trace::TraceRawVcs, Completion, RcStr, TryJoinIterExt, Value, Vc};
use turbo_tasks::{
trace::TraceRawVcs, Completion, RcStr, TryJoinIterExt, Value, ValueToString, Vc,
};
use turbo_tasks_env::{CustomProcessEnv, ProcessEnv};
use turbo_tasks_fs::{File, FileContent, FileSystemPath};
use turbopack::{
Expand Down Expand Up @@ -686,6 +689,11 @@ fn client_shared_chunks() -> Vc<RcStr> {
Vc::cell("client_shared_chunks".into())
}

#[turbo_tasks::function]
fn server_utils_module() -> Vc<RcStr> {
Vc::cell("server-utils".into())
}

#[derive(Copy, Clone, Serialize, Deserialize, PartialEq, Eq, Debug, TraceRawVcs)]
enum AppPageEndpointType {
Html,
Expand Down Expand Up @@ -1256,31 +1264,62 @@ impl AppEndpoint {
let mut current_chunks = OutputAssets::empty();
let mut current_availability_info = AvailabilityInfo::Root;
if let Some(client_references) = client_references {
for server_component in client_references
.await?
.server_component_entries
.iter()
.copied()
{
let server_path = server_component.server_path();
let is_layout =
server_path.file_stem().await?.as_deref() == Some("layout");
let client_references = client_references.await?;
let span = tracing::trace_span!("server utils",);
async {
let utils_module = IncludeModulesModule::new(
AssetIdent::from_path(this.app_project.project().project_path())
.with_modifier(server_utils_module()),
client_references.server_utils.clone(),
);

let chunk_group = chunking_context
.chunk_group(
server_component.ident(),
Vc::upcast(server_component),
utils_module.ident(),
Vc::upcast(utils_module),
Value::new(current_availability_info),
)
.await?;

if is_layout {
current_chunks = current_chunks
.concatenate(chunk_group.assets)
.resolve()
.await?;
current_availability_info = chunk_group.availability_info;

anyhow::Ok(())
}
.instrument(span)
.await?;
for server_component in client_references
.server_component_entries
.iter()
.copied()
.take(client_references.server_component_entries.len() - 1)
{
let span = tracing::trace_span!(
"layout segment",
name = server_component.ident().to_string().await?.as_str()
);
async {
let chunk_group = chunking_context
.chunk_group(
server_component.ident(),
Vc::upcast(server_component),
Value::new(current_availability_info),
)
.await?;

current_chunks = current_chunks
.concatenate(chunk_group.assets)
.resolve()
.await?;
current_availability_info = chunk_group.availability_info;

anyhow::Ok(())
}
.instrument(span)
.await?;
}
}
chunking_context
Expand Down
80 changes: 32 additions & 48 deletions crates/next-core/src/app_page_loader_tree.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,10 @@ use std::{

use anyhow::Result;
use indexmap::IndexMap;
use turbo_tasks::{RcStr, Value, Vc};
use turbo_tasks::{RcStr, Vc};
use turbo_tasks_fs::FileSystemPath;
use turbopack::{transition::Transition, ModuleAssetContext};
use turbopack_core::{
context::AssetContext,
file_source::FileSource,
module::Module,
reference_type::{EcmaScriptModulesReferenceSubType, InnerAssets, ReferenceType},
};
use turbopack_core::{file_source::FileSource, module::Module};
use turbopack_ecmascript::{magic_identifier, text::TextContentFileSource, utils::StringifyJs};

use crate::{
Expand Down Expand Up @@ -193,16 +188,7 @@ impl AppPageLoaderTreeBuilder {
app_page.clone(),
);

let module = self
.base
.module_asset_context
.process(
source,
Value::new(ReferenceType::EcmaScriptModules(
EcmaScriptModulesReferenceSubType::Undefined,
)),
)
.module();
let module = self.base.process_source(source);
self.base
.inner_assets
.insert(inner_module_id.into(), module);
Expand Down Expand Up @@ -237,14 +223,15 @@ impl AppPageLoaderTreeBuilder {
self.base
.imports
.push(format!("import {identifier} from \"{inner_module_id}\";").into());
self.base.inner_assets.insert(
inner_module_id.into(),
Vc::upcast(StructuredImageModuleType::create_module(
Vc::upcast(FileSource::new(path)),
BlurPlaceholderMode::None,
self.base.module_asset_context,
)),
);
let module = Vc::upcast(StructuredImageModuleType::create_module(
Vc::upcast(FileSource::new(path)),
BlurPlaceholderMode::None,
self.base.module_asset_context,
));
let module = self.base.process_module(module);
self.base
.inner_assets
.insert(inner_module_id.into(), module);

let s = " ";
writeln!(self.loader_tree_code, "{s}(async (props) => [{{")?;
Expand Down Expand Up @@ -286,14 +273,9 @@ impl AppPageLoaderTreeBuilder {

let module = self
.base
.module_asset_context
.process(
Vc::upcast(TextContentFileSource::new(Vc::upcast(FileSource::new(
alt_path,
)))),
Value::new(ReferenceType::Internal(InnerAssets::empty())),
)
.module();
.process_source(Vc::upcast(TextContentFileSource::new(Vc::upcast(
FileSource::new(alt_path),
))));

self.base
.inner_assets
Expand Down Expand Up @@ -339,12 +321,18 @@ impl AppPageLoaderTreeBuilder {
route: _,
} = &modules;

// Ensure global metadata being written only once at the root level
// Otherwise child pages will have redundant metadata
let global_metadata = &*global_metadata.await?;
self.write_metadata(
app_page,
metadata,
if root { Some(global_metadata) } else { None },
)
.await?;

self.write_modules_entry(AppDirModuleType::Layout, *layout)
.await?;
self.write_modules_entry(AppDirModuleType::Page, *page)
.await?;
self.write_modules_entry(AppDirModuleType::DefaultPage, *default)
.await?;
self.write_modules_entry(AppDirModuleType::Error, *error)
.await?;
self.write_modules_entry(AppDirModuleType::Loading, *loading)
Expand All @@ -353,6 +341,10 @@ impl AppPageLoaderTreeBuilder {
.await?;
self.write_modules_entry(AppDirModuleType::NotFound, *not_found)
.await?;
self.write_modules_entry(AppDirModuleType::Page, *page)
.await?;
self.write_modules_entry(AppDirModuleType::DefaultPage, *default)
.await?;

let modules_code = replace(&mut self.loader_tree_code, temp_loader_tree_code);

Expand All @@ -366,16 +358,6 @@ impl AppPageLoaderTreeBuilder {

self.loader_tree_code += &modules_code;

// Ensure global metadata being written only once at the root level
// Otherwise child pages will have redundant metadata
let global_metadata = &*global_metadata.await?;
self.write_metadata(
app_page,
metadata,
if root { Some(global_metadata) } else { None },
)
.await?;

write!(self.loader_tree_code, "}}]")?;
Ok(())
}
Expand All @@ -388,7 +370,9 @@ impl AppPageLoaderTreeBuilder {

let modules = &loader_tree.modules;
if let Some(global_error) = modules.global_error {
let module = self.base.process_module(global_error);
let module = self
.base
.process_source(Vc::upcast(FileSource::new(global_error)));
self.base.inner_assets.insert(GLOBAL_ERROR.into(), module);
};

Expand Down
12 changes: 8 additions & 4 deletions crates/next-core/src/base_loader_tree.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use turbopack_core::{
file_source::FileSource,
module::Module,
reference_type::{EcmaScriptModulesReferenceSubType, ReferenceType},
source::Source,
};
use turbopack_ecmascript::{magic_identifier, utils::StringifyJs};

Expand Down Expand Up @@ -64,9 +65,7 @@ impl BaseLoaderTreeBuilder {
i
}

pub fn process_module(&self, path: Vc<FileSystemPath>) -> Vc<Box<dyn Module>> {
let source = Vc::upcast(FileSource::new(path));

pub fn process_source(&self, source: Vc<Box<dyn Source>>) -> Vc<Box<dyn Module>> {
let reference_type = Value::new(ReferenceType::EcmaScriptModules(
EcmaScriptModulesReferenceSubType::Undefined,
));
Expand All @@ -76,6 +75,11 @@ impl BaseLoaderTreeBuilder {
.module()
}

pub fn process_module(&self, module: Vc<Box<dyn Module>>) -> Vc<Box<dyn Module>> {
self.server_component_transition
.process_module(module, self.module_asset_context)
}

pub async fn create_module_tuple_code(
&mut self,
module_type: AppDirModuleType,
Expand All @@ -96,7 +100,7 @@ impl BaseLoaderTreeBuilder {
.into(),
);

let module = self.process_module(path);
let module = self.process_source(Vc::upcast(FileSource::new(path)));

self.inner_assets
.insert(format!("MODULE_{i}").into(), module);
Expand Down
2 changes: 1 addition & 1 deletion crates/next-core/src/next_app/app_page_entry.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@
let source = VirtualSource::new_with_ident(
source
.ident()
.with_query(Vc::cell(query.to_string().into())),
.with_query(Vc::cell(format!("?{}", query.to_string()).into())),

Check failure on line 110 in crates/next-core/src/next_app/app_page_entry.rs

View workflow job for this annotation

GitHub Actions / rust check / build 

`to_string` applied to a type that implements `Display` in `format!` args
sokra marked this conversation as resolved.
Show resolved Hide resolved
AssetContent::file(file.into()),
);

Expand Down
Loading
Loading