Skip to content

[turbopack] Simplify Request::parse(Pattern) the recursive portion doesn't need to be async #80510

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
lukesandberg merged 3 commits into from
Jun 16, 2025
Merged

[turbopack] Simplify Request::parse(Pattern) the recursive portion doesn't need to be async #80510

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
048978b
simplify `Request::parse(Pattern)` the recursive portion doesn't need…
lukesandberg Jun 13, 2025
98f622a
fix formatting and inline the alternates function
lukesandberg Jun 14, 2025
13b21d2
fix typo
lukesandberg Jun 14, 2025
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
108 changes: 60 additions & 48 deletions turbopack/crates/turbopack-core/src/resolve/parse.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,88 +142,91 @@ impl Request {
})
}

pub async fn parse_ref(mut request: Pattern) -> Result<Self> {
request.normalize();
Ok(match request {
/// Internal construction function. Should only be called with normalized patterns, or
/// recursively. Most users should call [Self::parse] instead.
fn parse_ref(request: Pattern) -> Self {
Comment thread
lukesandberg marked this conversation as resolved.
match request {
Pattern::Dynamic => Request::Dynamic,
Pattern::Constant(r) => Request::parse_constant_pattern(r).await?,
Pattern::Concatenation(list) => Request::parse_concatenation_pattern(list).await?,
Pattern::Alternatives(list) => Request::parse_alternatives_pattern(list).await?,
})
Pattern::Constant(r) => Request::parse_constant_pattern(r),
Pattern::Concatenation(list) => Request::parse_concatenation_pattern(list),
Pattern::Alternatives(_) => panic!(
"request should be normalized and alternatives should have already been handled.",
),
}
}

async fn parse_constant_pattern(r: RcStr) -> Result<Self> {
fn parse_constant_pattern(r: RcStr) -> Self {
if r.is_empty() {
return Ok(Request::Empty);
return Request::Empty;
}

if let Some(remainder) = r.strip_prefix("//") {
return Ok(Request::Uri {
return Request::Uri {
protocol: rcstr!("//"),
remainder: remainder.into(),
query: RcStr::default(),
fragment: RcStr::default(),
});
};
}

if r.starts_with('/') {
let (path, query, fragment) = split_off_query_fragment(&r);

return Ok(Request::ServerRelative {
return Request::ServerRelative {
path,
query,
fragment,
});
};
}

if r.starts_with('#') {
return Ok(Request::PackageInternal {
return Request::PackageInternal {
path: Pattern::Constant(r),
});
};
}

if r.starts_with("./") || r.starts_with("../") || r == "." || r == ".." {
let (path, query, fragment) = split_off_query_fragment(&r);

return Ok(Request::Relative {
return Request::Relative {
path,
force_in_lookup_dir: false,
query,
fragment,
});
};
}

if WINDOWS_PATH.is_match(&r) {
let (path, query, fragment) = split_off_query_fragment(&r);

return Ok(Request::Windows {
return Request::Windows {
path,
query,
fragment,
});
};
}

if let Some(caps) = URI_PATH.captures(&r)
&& let (Some(protocol), Some(remainder)) = (caps.get(1), caps.get(2))
{
if let Some(caps) = DATA_URI_REMAINDER.captures(remainder.as_str()) {
let media_type = caps.get(1).map_or("", |m| m.as_str()).into();
let encoding = caps.get(2).map_or("", |e| e.as_str()).into();
let data = caps.get(3).map_or("", |d| d.as_str()).into();
let media_type = caps.get(1).map_or(RcStr::default(), |m| m.as_str().into());
let encoding = caps.get(2).map_or(RcStr::default(), |e| e.as_str().into());
let data = caps.get(3).map_or(RcStr::default(), |d| d.as_str().into());

return Ok(Request::DataUri {
return Request::DataUri {
media_type,
encoding,
data: ResolvedVc::cell(data),
});
};
}

return Ok(Request::Uri {
return Request::Uri {
protocol: protocol.as_str().into(),
remainder: remainder.as_str().into(),
query: RcStr::default(),
fragment: RcStr::default(),
});
};
}

if let Some((module, path)) = MODULE_PATH
Expand All @@ -232,25 +235,25 @@ impl Request {
{
let (path, query, fragment) = split_off_query_fragment(path.as_str());

return Ok(Request::Module {
return Request::Module {
module: module.as_str().into(),
path,
query,
fragment,
});
};
}

Ok(Request::Unknown {
Request::Unknown {
path: Pattern::Constant(r),
})
}
}

async fn parse_concatenation_pattern(list: Vec<Pattern>) -> Result<Self> {
fn parse_concatenation_pattern(list: Vec<Pattern>) -> Self {
if list.is_empty() {
return Ok(Request::Empty);
return Request::Empty;
}

let mut result = Box::pin(Self::parse_ref(list[0].clone())).await?;
let mut result = Self::parse_ref(list[0].clone());

for item in list.into_iter().skip(1) {
match &mut result {
Expand All @@ -270,7 +273,7 @@ impl Request {
path.push(item);
}
Request::Empty => {
result = Box::pin(Self::parse_ref(item)).await?;
result = Self::parse_ref(item);
}
Request::PackageInternal { path } => {
path.push(item);
Expand All @@ -289,30 +292,39 @@ impl Request {
};
}

Ok(result)
}

async fn parse_alternatives_pattern(list: Vec<Pattern>) -> Result<Self> {
Ok(Request::Alternatives {
requests: list
.into_iter()
.map(Request::parse)
.map(|v| async move { v.to_resolved().await })
.try_join()
.await?,
})
result
}

pub fn parse_string(request: RcStr) -> Vc<Self> {
Self::parse(request.into())
}

pub fn parse(mut request: Pattern) -> Vc<Self> {
// Call normalize before parse_inner to improve cache hits.
request.normalize();
Self::parse_inner(request)
}
}

#[turbo_tasks::value_impl]
impl Request {
#[turbo_tasks::function]
pub async fn parse(request: Pattern) -> Result<Vc<Self>> {
Ok(Self::cell(Request::parse_ref(request).await?))
async fn parse_inner(request: Pattern) -> Result<Vc<Self>> {
// Because we are normalized, we should handle alternatives here
if let Pattern::Alternatives(alts) = request {
Ok(Self::cell(Self::Alternatives {
requests: alts
.into_iter()
// We can call parse_inner directly because these patterns are already
// normalized. We don't call `Self::parse_ref` so we can try to get a cache hit
// on the sub-patterns
.map(|p| Self::parse_inner(p).to_resolved())
.try_join()
.await?,
}))
} else {
Ok(Self::cell(Self::parse_ref(request)))
}
}

#[turbo_tasks::function]
Expand Down
19 changes: 13 additions & 6 deletions turbopack/crates/turbopack-core/src/resolve/pattern.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use regex::Regex;
use rustc_hash::{FxHashMap, FxHashSet};
use serde::{Deserialize, Serialize};
use tracing::Instrument;
use turbo_rcstr::RcStr;
use turbo_rcstr::{RcStr, rcstr};
use turbo_tasks::{
NonLocalValue, ResolvedVc, TaskInput, ValueToString, Vc, debug::ValueDebugFormat,
trace::TraceRawVcs,
Expand Down Expand Up @@ -448,13 +448,20 @@ impl Pattern {
current
}

/// Normalizes paths by
/// - processing path segments: `.` and `..`
/// - normalizing windows filepaths by replacing `\` with `/`
///
/// The Pattern must have already been processed by [Self::normalize].
/// Returns [Option::None] if any of the patterns attempt to navigate out of the root.
pub fn with_normalized_path(&self) -> Option<Pattern> {
let mut new = self.clone();

fn normalize_path_internal(pattern: &mut Pattern) -> Option<()> {
match pattern {
Pattern::Constant(c) => {
*c = (*(normalize_path(c)?)).into();
let normalized = c.replace('\\', "/");
*c = (*(normalize_path(normalized.as_str())?)).into();
Some(())
}
Pattern::Dynamic => Some(()),
Expand Down Expand Up @@ -490,7 +497,7 @@ impl Pattern {
}
}
}
let separator: RcStr = "/".into();
let separator = rcstr!("/");
*list = segments
.into_iter()
.flat_map(|c| {
Expand Down Expand Up @@ -1091,7 +1098,7 @@ impl Pattern {

pub fn or_any_nested_file(&self) -> Self {
let mut new = self.clone();
new.push(Pattern::Constant("/".into()));
new.push(Pattern::Constant(rcstr!("/")));
new.push(Pattern::Dynamic);
new.normalize();
Pattern::alternatives([self.clone(), new])
Expand Down Expand Up @@ -1548,10 +1555,10 @@ pub async fn read_matches(
}
if prefix.is_empty() {
if let Some(pos) = pat.match_position("./") {
results.push((pos, PatternMatch::Directory("./".into(), lookup_dir)));
results.push((pos, PatternMatch::Directory(rcstr!("./"), lookup_dir)));
}
if let Some(pos) = pat.could_match_position("./") {
nested.push((pos, read_matches(*lookup_dir, "./".into(), false, pattern)));
nested.push((pos, read_matches(*lookup_dir, rcstr!("./"), false, pattern)));
}
} else {
prefix.push('/');
Expand Down