Skip to content

feat(turbopack): optimize resolve plugin conditions with Option #92

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
xusd320 merged 1 commit into from
Jan 7, 2026
Merged

feat(turbopack): optimize resolve plugin conditions with Option #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 11 additions & 6 deletions crates/next-core/src/next_font/local/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ use turbopack_core::{
resolve::{
ResolveResult, ResolveResultItem, ResolveResultOption,
parse::Request,
plugin::{BeforeResolvePlugin, BeforeResolvePluginCondition},
plugin::{
BeforeResolvePlugin, BeforeResolvePluginCondition, OptionBeforeResolvePluginCondition,
},
},
virtual_source::VirtualSource,
};
Expand Down Expand Up @@ -64,11 +66,14 @@ impl NextFontLocalResolvePlugin {
#[turbo_tasks::value_impl]
impl BeforeResolvePlugin for NextFontLocalResolvePlugin {
#[turbo_tasks::function]
fn before_resolve_condition(&self) -> Vc<BeforeResolvePluginCondition> {
BeforeResolvePluginCondition::from_request_glob(Glob::new(
rcstr!("{next,@vercel/turbopack-next/internal}/font/local/*"),
GlobOptions::default(),
))
fn before_resolve_condition(&self) -> Vc<OptionBeforeResolvePluginCondition> {
OptionBeforeResolvePluginCondition::some(
BeforeResolvePluginCondition::from_request_glob(Glob::new(
rcstr!("{next,@vercel/turbopack-next/internal}/font/local/*"),
GlobOptions::default(),
))
.to_resolved(),
)
}

#[turbo_tasks::function]
Expand Down
17 changes: 11 additions & 6 deletions crates/next-core/src/next_server/resolve.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ use turbopack_core::{
package_json,
parse::Request,
pattern::Pattern,
plugin::{AfterResolvePlugin, AfterResolvePluginCondition},
plugin::{
AfterResolvePlugin, AfterResolvePluginCondition, OptionAfterResolvePluginCondition,
},
resolve,
},
source::Source,
Expand Down Expand Up @@ -64,17 +66,20 @@ impl ExternalCjsModulesResolvePlugin {
}

#[turbo_tasks::function]
fn condition(root: FileSystemPath) -> Vc<AfterResolvePluginCondition> {
AfterResolvePluginCondition::new(
root,
Glob::new(rcstr!("**/node_modules/**"), GlobOptions::default()),
fn condition(root: FileSystemPath) -> Vc<OptionAfterResolvePluginCondition> {
OptionAfterResolvePluginCondition::some(
AfterResolvePluginCondition::new(
root,
Glob::new(rcstr!("**/node_modules/**"), GlobOptions::default()),
)
.to_resolved(),
)
}

#[turbo_tasks::value_impl]
impl AfterResolvePlugin for ExternalCjsModulesResolvePlugin {
#[turbo_tasks::function]
fn after_resolve_condition(&self) -> Vc<AfterResolvePluginCondition> {
fn after_resolve_condition(&self) -> Vc<OptionAfterResolvePluginCondition> {
condition(self.root.clone())
}

Expand Down
78 changes: 47 additions & 31 deletions crates/next-core/src/next_shared/resolve.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ use turbopack_core::{
pattern::Pattern,
plugin::{
AfterResolvePlugin, AfterResolvePluginCondition, BeforeResolvePlugin,
BeforeResolvePluginCondition,
BeforeResolvePluginCondition, OptionAfterResolvePluginCondition,
OptionBeforeResolvePluginCondition,
},
},
};
Expand Down Expand Up @@ -128,8 +129,11 @@ impl InvalidImportResolvePlugin {
#[turbo_tasks::value_impl]
impl BeforeResolvePlugin for InvalidImportResolvePlugin {
#[turbo_tasks::function]
fn before_resolve_condition(&self) -> Vc<BeforeResolvePluginCondition> {
BeforeResolvePluginCondition::from_modules(Vc::cell(vec![self.invalid_import.clone()]))
fn before_resolve_condition(&self) -> Vc<OptionBeforeResolvePluginCondition> {
OptionBeforeResolvePluginCondition::some(
BeforeResolvePluginCondition::from_modules(Vc::cell(vec![self.invalid_import.clone()]))
.to_resolved(),
)
}

#[turbo_tasks::function]
Expand Down Expand Up @@ -223,13 +227,16 @@ impl NextExternalResolvePlugin {
#[turbo_tasks::value_impl]
impl AfterResolvePlugin for NextExternalResolvePlugin {
#[turbo_tasks::function]
async fn after_resolve_condition(&self) -> Result<Vc<AfterResolvePluginCondition>> {
Ok(AfterResolvePluginCondition::new(
self.project_path.root().owned().await?,
Glob::new(
rcstr!("**/next/dist/**/*.{external,runtime.dev,runtime.prod}.js"),
GlobOptions::default(),
),
async fn after_resolve_condition(&self) -> Result<Vc<OptionAfterResolvePluginCondition>> {
Ok(OptionAfterResolvePluginCondition::some(
AfterResolvePluginCondition::new(
self.project_path.root().owned().await?,
Glob::new(
rcstr!("**/next/dist/**/*.{external,runtime.dev,runtime.prod}.js"),
GlobOptions::default(),
),
)
.to_resolved(),
))
}

Expand Down Expand Up @@ -281,13 +288,16 @@ impl NextNodeSharedRuntimeResolvePlugin {
#[turbo_tasks::value_impl]
impl AfterResolvePlugin for NextNodeSharedRuntimeResolvePlugin {
#[turbo_tasks::function]
async fn after_resolve_condition(&self) -> Result<Vc<AfterResolvePluginCondition>> {
Ok(AfterResolvePluginCondition::new(
self.root.root().owned().await?,
Glob::new(
rcstr!("**/next/dist/**/*.shared-runtime.js"),
GlobOptions::default(),
),
async fn after_resolve_condition(&self) -> Result<Vc<OptionAfterResolvePluginCondition>> {
Ok(OptionAfterResolvePluginCondition::some(
AfterResolvePluginCondition::new(
self.root.root().owned().await?,
Glob::new(
rcstr!("**/next/dist/**/*.shared-runtime.js"),
GlobOptions::default(),
),
)
.to_resolved(),
))
}

Expand Down Expand Up @@ -352,13 +362,16 @@ impl ModuleFeatureReportResolvePlugin {
#[turbo_tasks::value_impl]
impl BeforeResolvePlugin for ModuleFeatureReportResolvePlugin {
#[turbo_tasks::function]
fn before_resolve_condition(&self) -> Vc<BeforeResolvePluginCondition> {
BeforeResolvePluginCondition::from_modules(Vc::cell(
FEATURE_MODULES
.keys()
.map(|k| (*k).into())
.collect::<Vec<RcStr>>(),
))
fn before_resolve_condition(&self) -> Vc<OptionBeforeResolvePluginCondition> {
OptionBeforeResolvePluginCondition::some(
BeforeResolvePluginCondition::from_modules(Vc::cell(
FEATURE_MODULES
.keys()
.map(|k| (*k).into())
.collect::<Vec<RcStr>>(),
))
.to_resolved(),
)
}

#[turbo_tasks::function]
Expand Down Expand Up @@ -409,13 +422,16 @@ impl NextSharedRuntimeResolvePlugin {
#[turbo_tasks::value_impl]
impl AfterResolvePlugin for NextSharedRuntimeResolvePlugin {
#[turbo_tasks::function]
async fn after_resolve_condition(&self) -> Result<Vc<AfterResolvePluginCondition>> {
Ok(AfterResolvePluginCondition::new(
self.root.root().owned().await?,
Glob::new(
rcstr!("**/next/dist/esm/**/*.shared-runtime.js"),
GlobOptions::default(),
),
async fn after_resolve_condition(&self) -> Result<Vc<OptionAfterResolvePluginCondition>> {
Ok(OptionAfterResolvePluginCondition::some(
AfterResolvePluginCondition::new(
self.root.root().owned().await?,
Glob::new(
rcstr!("**/next/dist/esm/**/*.shared-runtime.js"),
GlobOptions::default(),
),
)
.to_resolved(),
))
}

Expand Down
43 changes: 30 additions & 13 deletions turbopack/crates/turbopack-core/src/resolve/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1685,9 +1685,12 @@ async fn handle_before_resolve_plugins(
options: Vc<ResolveOptions>,
) -> Result<Option<Vc<ResolveResult>>> {
for plugin in &options.await?.before_resolve_plugins {
let condition = plugin.before_resolve_condition().resolve().await?;
if !*condition.matches(request).await? {
continue;
let condition_option = plugin.before_resolve_condition().await?;
if let Some(condition) = *condition_option {
let condition = condition.resolve().await?;
if !*condition.matches(request).await? {
continue;
}
}
Comment on lines +1688 to 1694
Copy link

@gemini-code-assist gemini-code-assist bot Jan 7, 2026

Choose a reason for hiding this comment

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

high

This block has a potential correctness issue and can be refactored for better readability.

  1. condition.resolve().await? resolves the Vc to an Arc<...>, but turbo_tasks::functions like matches should be called on the Vc itself.
  2. The logic can be flattened to avoid nested if statements by first determining if the condition matches, and then acting on it.

Here is a suggested refactoring that addresses both points.

Suggested change
let condition_option = plugin.before_resolve_condition().await?;
if let Some(condition) = *condition_option {
let condition = condition.resolve().await?;
if !*condition.matches(request).await? {
continue;
}
}
let condition_option = plugin.before_resolve_condition().await?;
let matches = if let Some(condition) = *condition_option {
*condition.resolve().matches(request).await?
} else {
true
};
if !matches {
continue;
}


if let Some(result) = *plugin
Expand Down Expand Up @@ -1716,17 +1719,31 @@ async fn handle_after_resolve_plugins(
options: Vc<ResolveOptions>,
) -> Result<Option<Vc<ResolveResult>>> {
for plugin in &options.await?.after_resolve_plugins {
let after_resolve_condition = plugin.after_resolve_condition().resolve().await?;
if *after_resolve_condition.matches(path.clone()).await?
&& let Some(result) = *plugin
.after_resolve(
path.clone(),
lookup_path.clone(),
reference_type.clone(),
request,
)
.await?
let after_resolve_condition_option = plugin.after_resolve_condition().await?;
if let Some(after_resolve_condition) = *after_resolve_condition_option {
let after_resolve_condition = after_resolve_condition.resolve().await?;
if *after_resolve_condition.matches(path.clone()).await?
&& let Some(result) = *plugin
.after_resolve(
path.clone(),
lookup_path.clone(),
reference_type.clone(),
request,
)
.await?
{
return Ok(Some(*result));
}
} else if let Some(result) = *plugin
.after_resolve(
path.clone(),
lookup_path.clone(),
reference_type.clone(),
request,
)
.await?
{
// If there is no condition, the plugin applies to all paths.
return Ok(Some(*result));
}
Comment on lines +1722 to 1748
Copy link

@gemini-code-assist gemini-code-assist bot Jan 7, 2026

Choose a reason for hiding this comment

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

high

This block has a potential correctness issue and can be significantly simplified to improve readability and reduce code duplication.

  1. Similar to the before_resolve handler, after_resolve_condition.resolve().await? is used, which is likely incorrect for calling a turbo_tasks::function. matches should be called on the Vc.
  2. The plugin.after_resolve(...) call is duplicated in both the if and else if branches.
  3. The overall logic can be flattened.

Here's a suggested refactoring that addresses these points.

            let after_resolve_condition_option = plugin.after_resolve_condition().await?;
            let matches = if let Some(after_resolve_condition) = *after_resolve_condition_option {
                *after_resolve_condition.resolve().matches(path.clone()).await?
            } else {
                // If there is no condition, the plugin applies to all paths.
                true
            };

            if matches {
                if let Some(result) = *plugin
                    .after_resolve(
                        path.clone(),
                        lookup_path.clone(),
                        reference_type.clone(),
                        request,
                    )
                    .await?
                {
                    return Ok(Some(*result));
                }
            }

}
Expand Down
36 changes: 34 additions & 2 deletions turbopack/crates/turbopack-core/src/resolve/plugin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,22 @@ impl AfterResolvePluginCondition {
}
}

#[turbo_tasks::value(transparent)]
pub struct OptionAfterResolvePluginCondition(Option<ResolvedVc<AfterResolvePluginCondition>>);

#[turbo_tasks::value_impl]
impl OptionAfterResolvePluginCondition {
#[turbo_tasks::function]
pub fn none() -> Vc<Self> {
Vc::cell(None)
}

#[turbo_tasks::function]
pub fn some(condition: ResolvedVc<AfterResolvePluginCondition>) -> Vc<Self> {
Vc::cell(Some(condition))
}
}

/// A condition which determines if the hooks of a resolve plugin gets called.
#[turbo_tasks::value]
pub enum BeforeResolvePluginCondition {
Expand Down Expand Up @@ -80,10 +96,26 @@ impl BeforeResolvePluginCondition {
}
}

#[turbo_tasks::value(transparent)]
pub struct OptionBeforeResolvePluginCondition(Option<ResolvedVc<BeforeResolvePluginCondition>>);

#[turbo_tasks::value_impl]
impl OptionBeforeResolvePluginCondition {
#[turbo_tasks::function]
pub fn none() -> Vc<Self> {
Vc::cell(None)
}

#[turbo_tasks::function]
pub fn some(condition: ResolvedVc<BeforeResolvePluginCondition>) -> Vc<Self> {
Vc::cell(Some(condition))
}
}

#[turbo_tasks::value_trait]
pub trait BeforeResolvePlugin {
#[turbo_tasks::function]
fn before_resolve_condition(self: Vc<Self>) -> Vc<BeforeResolvePluginCondition>;
fn before_resolve_condition(self: Vc<Self>) -> Vc<OptionBeforeResolvePluginCondition>;

#[turbo_tasks::function]
fn before_resolve(
Expand All @@ -98,7 +130,7 @@ pub trait BeforeResolvePlugin {
pub trait AfterResolvePlugin {
/// A condition which determines if the hooks gets called.
#[turbo_tasks::function]
fn after_resolve_condition(self: Vc<Self>) -> Vc<AfterResolvePluginCondition>;
fn after_resolve_condition(self: Vc<Self>) -> Vc<OptionAfterResolvePluginCondition>;

/// This hook gets called when a full filepath has been resolved and the
/// condition matches. If a value is returned it replaces the resolve
Expand Down
Loading