Skip to content

Commit

Permalink
feat(turbopack): add support for esm externals in app dir (#64918)
Browse files Browse the repository at this point in the history 
Writes the async flag to the client reference manifest to support ESM
externals in app dir.

The `app-ssr` context can't have esm externals as that might cause a
module to be async in ssr but not on the client.

Closes PACK-2967
Fixes #64525
  • Loading branch information
@ForsakenHarmony @lubieowoce
ForsakenHarmony authored and lubieowoce committed Sep 3, 2024
1 parent dafdc81 commit d09b769
Show file tree
Hide file tree
Showing 6 changed files with 183 additions and 122 deletions.
6 changes: 3 additions & 3 deletions packages/next-swc/crates/next-api/src/app.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -846,13 +846,13 @@ impl AppEndpoint {
{
entry_client_chunks.extend(chunks.await?.iter().copied());
}
for chunks in client_references_chunks_ref
for (chunks, _) in client_references_chunks_ref
.client_component_client_chunks
.values()
{
client_assets.extend(chunks.await?.iter().copied());
}
for chunks in client_references_chunks_ref
for (chunks, _) in client_references_chunks_ref
.client_component_ssr_chunks
.values()
{
Expand Down Expand Up @@ -929,7 +929,7 @@ impl AppEndpoint {
// initialization
let client_references_chunks = &*client_references_chunks.await?;

for &ssr_chunks in client_references_chunks
for (ssr_chunks, _) in client_references_chunks
.client_component_ssr_chunks
.values()
{
Expand Down
63 changes: 48 additions & 15 deletions packages/next-swc/crates/next-core/src/next_app/app_client_references_chunks.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@ fn client_modules_ssr_modifier() -> Vc<String> {

#[turbo_tasks::value]
pub struct ClientReferencesChunks {
pub client_component_client_chunks: IndexMap<ClientReferenceType, Vc<OutputAssets>>,
pub client_component_ssr_chunks: IndexMap<ClientReferenceType, Vc<OutputAssets>>,
pub client_component_client_chunks:
IndexMap<ClientReferenceType, (Vc<OutputAssets>, AvailabilityInfo)>,
pub client_component_ssr_chunks:
IndexMap<ClientReferenceType, (Vc<OutputAssets>, AvailabilityInfo)>,
pub layout_segment_client_chunks: IndexMap<Vc<NextServerComponentModule>, Vc<OutputAssets>>,
}

Expand Down Expand Up @@ -66,23 +68,47 @@ pub async fn get_app_client_references_chunks(
) => {
let ecmascript_client_reference_ref =
ecmascript_client_reference.await?;
(
client_chunking_context.root_chunk_group_assets(Vc::upcast(

let client_chunk_group = client_chunking_context
.root_chunk_group(Vc::upcast(
ecmascript_client_reference_ref.client_module,
)),
ssr_chunking_context.map(|ssr_chunking_context| {
ssr_chunking_context.root_chunk_group_assets(Vc::upcast(
ecmascript_client_reference_ref.ssr_module,
))
.await?;

(
(
client_chunk_group.assets,
client_chunk_group.availability_info,
),
if let Some(ssr_chunking_context) = ssr_chunking_context {
let ssr_chunk_group = ssr_chunking_context
.root_chunk_group(Vc::upcast(
ecmascript_client_reference_ref.ssr_module,
))
.await?;

Some((
ssr_chunk_group.assets,
ssr_chunk_group.availability_info,
))
}),
} else {
None
},
)
}
ClientReferenceType::CssClientReference(css_client_reference) => {
let css_client_reference_ref = css_client_reference.await?;
(
client_chunking_context.root_chunk_group_assets(Vc::upcast(
let client_chunk_group = client_chunking_context
.root_chunk_group(Vc::upcast(
css_client_reference_ref.client_module,
)),
))
.await?;

(
(
client_chunk_group.assets,
client_chunk_group.availability_info,
),
None,
)
}
Expand Down Expand Up @@ -165,6 +191,7 @@ pub async fn get_app_client_references_chunks(
})
.try_flat_join()
.await?;

let ssr_chunk_group = if !ssr_modules.is_empty() {
let ssr_entry_module = IncludeModulesModule::new(
base_ident.with_modifier(client_modules_ssr_modifier()),
Expand All @@ -184,6 +211,7 @@ pub async fn get_app_client_references_chunks(
} else {
None
};

let client_modules = client_reference_types
.iter()
.map(|client_reference_ty| async move {
Expand Down Expand Up @@ -240,8 +268,10 @@ pub async fn get_app_client_references_chunks(
if let ClientReferenceType::EcmascriptClientReference(_) =
client_reference_ty
{
client_component_client_chunks
.insert(client_reference_ty, client_chunks);
client_component_client_chunks.insert(
client_reference_ty,
(client_chunks, client_chunk_group.availability_info),
);
}
}
}
Expand All @@ -261,7 +291,10 @@ pub async fn get_app_client_references_chunks(
if let ClientReferenceType::EcmascriptClientReference(_) =
client_reference_ty
{
client_component_ssr_chunks.insert(client_reference_ty, ssr_chunks);
client_component_ssr_chunks.insert(
client_reference_ty,
(ssr_chunks, ssr_chunk_group.availability_info),
);
}
}
}
Expand Down
118 changes: 75 additions & 43 deletions packages/next-swc/crates/next-core/src/next_manifests/client_reference_manifest.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ use turbo_tasks_fs::{File, FileSystemPath};
use turbopack_binding::turbopack::{
core::{
asset::AssetContent,
chunk::{ChunkItemExt, ChunkableModule, ModuleId as TurbopackModuleId},
chunk::{
availability_info::AvailabilityInfo, ChunkItem, ChunkItemExt, ChunkableModule,
ModuleId as TurbopackModuleId,
},
output::OutputAsset,
virtual_output::VirtualOutputAsset,
},
Expand Down Expand Up @@ -66,61 +69,70 @@ impl ClientReferenceManifest {
.to_string()
.await?;

let client_module_id = ecmascript_client_reference
let client_chunk_item = ecmascript_client_reference
.client_module
.as_chunk_item(Vc::upcast(client_chunking_context))
.id()
.await?;
.as_chunk_item(Vc::upcast(client_chunking_context));

let client_module_id = client_chunk_item.id().await?;

let (client_chunks_paths, client_is_async) =
if let Some((client_chunks, client_availability_info)) =
client_references_chunks
.client_component_client_chunks
.get(&app_client_reference_ty)
{
let client_chunks = client_chunks.await?;
let client_chunks_paths = client_chunks
.iter()
.map(|chunk| chunk.ident().path())
.try_join()
.await?;

let chunk_paths = client_chunks_paths
.iter()
.filter_map(|chunk_path| client_relative_path.get_path_to(chunk_path))
.map(ToString::to_string)
// It's possible that a chunk also emits CSS files, that will
// be handled separatedly.
.filter(|path| path.ends_with(".js"))
// .map(RcStr::from) // BACKPORT: no RcStr
.collect::<Vec<_>>();

let is_async =
is_item_async(client_availability_info, client_chunk_item).await?;

(chunk_paths, is_async)
} else {
(Vec::new(), false)
};

let client_chunks_paths = if let Some(client_chunks) = client_references_chunks
.client_component_client_chunks
.get(&app_client_reference_ty)
{
let client_chunks = client_chunks.await?;
let client_chunks_paths = client_chunks
.iter()
.map(|chunk| chunk.ident().path())
.try_join()
.await?;

client_chunks_paths
.iter()
.filter_map(|chunk_path| client_relative_path.get_path_to(chunk_path))
.map(ToString::to_string)
// It's possible that a chunk also emits CSS files, that will
// be handled separatedly.
.filter(|path| path.ends_with(".js"))
.collect::<Vec<_>>()
} else {
Vec::new()
};
entry_manifest.client_modules.module_exports.insert(
get_client_reference_module_key(&server_path, "*"),
ManifestNodeEntry {
name: "*".to_string(),
id: (&*client_module_id).into(),
chunks: client_chunks_paths,
// TODO(WEB-434)
r#async: false,
r#async: client_is_async,
},
);

if let Some(ssr_chunking_context) = ssr_chunking_context {
let ssr_module_id = ecmascript_client_reference
let ssr_chunk_item = ecmascript_client_reference
.ssr_module
.as_chunk_item(Vc::upcast(ssr_chunking_context))
.id()
.await?;
.as_chunk_item(Vc::upcast(ssr_chunking_context));

let ssr_chunks_paths = if runtime == NextRuntime::Edge {
let ssr_module_id = ssr_chunk_item.id().await?;

let (ssr_chunks_paths, ssr_is_async) = if runtime == NextRuntime::Edge {
// the chunks get added to the middleware-manifest.json instead
// of this file because the
// edge runtime doesn't support dynamically
// loading chunks.
Vec::new()
} else if let Some(ssr_chunks) = client_references_chunks
.client_component_ssr_chunks
.get(&app_client_reference_ty)
(Vec::new(), false)
} else if let Some((ssr_chunks, ssr_availability_info)) =
client_references_chunks
.client_component_ssr_chunks
.get(&app_client_reference_ty)
{
let ssr_chunks = ssr_chunks.await?;

Expand All @@ -130,23 +142,28 @@ impl ClientReferenceManifest {
.try_join()
.await?;

ssr_chunks_paths
let chunk_paths = ssr_chunks_paths
.iter()
.filter_map(|chunk_path| node_root_ref.get_path_to(chunk_path))
.map(ToString::to_string)
.collect::<Vec<_>>()
// .map(RcStr::from) // BACKPORT: no RcStr
.collect::<Vec<_>>();

let is_async = is_item_async(ssr_availability_info, ssr_chunk_item).await?;

(chunk_paths, is_async)
} else {
Vec::new()
(Vec::new(), false)
};

let mut ssr_manifest_node = ManifestNode::default();
ssr_manifest_node.module_exports.insert(
"*".to_string(),
ManifestNodeEntry {
name: "*".to_string(),
id: (&*ssr_module_id).into(),
chunks: ssr_chunks_paths,
// TODO(WEB-434)
r#async: false,
r#async: ssr_is_async,
},
);

Expand Down Expand Up @@ -250,3 +267,18 @@ pub fn get_client_reference_module_key(server_path: &str, export_name: &str) ->
format!("{}#{}", server_path, export_name)
}
}

async fn is_item_async(
availability_info: &AvailabilityInfo,
chunk_item: Vc<Box<dyn ChunkItem>>,
) -> Result<bool> {
let Some(available_chunk_items) = availability_info.available_chunk_items() else {
return Ok(false);
};

let Some(info) = &*available_chunk_items.get(chunk_item).await? else {
return Ok(false);
};

Ok(info.is_async)
}
7 changes: 4 additions & 3 deletions packages/next-swc/crates/next-core/src/next_server/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,15 @@ pub async fn get_server_resolve_options_context(
.cloned(),
);

let ty = ty.into_value();

let server_component_externals_plugin = ExternalCjsModulesResolvePlugin::new(
project_path,
project_path.root(),
ExternalPredicate::Only(Vc::cell(external_packages)).cell(),
// TODO(sokra) esmExternals support
false,
// app-ssr can't have esm externals as that would make the module async on the server only
*next_config.import_externals().await? && !matches!(ty, ServerContextType::AppSSR { .. }),
);
let ty = ty.into_value();

let mut custom_conditions = vec![mode.await?.condition().to_string()];
custom_conditions.extend(
Expand Down
9 changes: 3 additions & 6 deletions test/e2e/app-dir/app-external/app-external.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ async function resolveStreamResponse(response: any, onData?: any) {
}

describe('app dir - external dependency', () => {
const { next, skipped, isTurbopack } = nextTestSetup({
const { next, skipped } = nextTestSetup({
files: __dirname,
dependencies: {
swr: 'latest',
Expand Down Expand Up @@ -46,7 +46,7 @@ describe('app dir - external dependency', () => {
expect(result).toContain('Server subpath: subpath.default')
expect(result).toContain('Client: index.default')
expect(result).toContain('Client subpath: subpath.default')
expect(result).toContain('opt-out-react-version: 18.3.1')
expect(result).not.toContain('opt-out-react-version: 18.3.0-canary')
})
})

Expand Down Expand Up @@ -286,10 +286,7 @@ describe('app dir - external dependency', () => {
browser.elementByCss('#dual-pkg-outout button').click()
await check(async () => {
const text = await browser.elementByCss('#dual-pkg-outout p').text()
// TODO: enable esm externals for app router in turbopack for actions
expect(text).toBe(
isTurbopack ? 'dual-pkg-optout:cjs' : 'dual-pkg-optout:mjs'
)
expect(text).toBe('dual-pkg-optout:mjs')
return 'success'
}, /success/)
})
Expand Down
Loading

0 comments on commit d09b769

Please sign in to comment.