Skip to content

Commit

Permalink
Turbopack: uncell RequireContextValue (vercel#75683)
Browse files Browse the repository at this point in the history
This might make a difference if the `RequireContextMap` is huge and used
across many files

More importantly, this seems to solve this failure in the testing
backend on vercel#75440:

```
called 'Result::unwrap()' on an 'Err' value: Cell is empty
Stack backtrace:
0: <anyhow::Error>::msg::<&str>
1: <turbo_tasks::backend::TypedCellContent>::cast::<turbopack_emascript::analyzer::RequireContextValue>
2: <turbo_tasks::vc::read::ReadVcFuture<turbopack_ecmascript::analyzer::RequireContextValue> as core::future::future::Future>::poll
3: turbopack_ecmascript::analyzer::well_known::replace_well_known::{closure#0}
4: turbopack_ecmascript::analyzer::linker::visit::<mod::analyzer::bench_link::{closure#0}::{closure#0}::{closure#0}::{closure#0}, turbopack_emascript: analyzer: it {closure#0}>::{closure#0}
```
  • Loading branch information
mischnic authored Feb 5, 2025
1 parent cde0f93 commit 1bdc679
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 29 deletions.
23 changes: 10 additions & 13 deletions turbopack/crates/turbopack-ecmascript/src/analyzer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use swc_core::{
},
};
use turbo_rcstr::RcStr;
use turbo_tasks::{FxIndexMap, FxIndexSet, ResolvedVc, Vc};
use turbo_tasks::{FxIndexMap, FxIndexSet, Vc};
use turbopack_core::compile_time_info::{
CompileTimeDefineValue, DefineableNameSegment, FreeVarReference,
};
Expand Down Expand Up @@ -3755,21 +3755,18 @@ pub fn parse_require_context(args: &[JsValue]) -> Result<RequireContextOptions>
})
}

#[turbo_tasks::value(transparent)]
#[derive(Debug, Clone)]
#[derive(Debug, Clone, Eq, PartialEq)]
pub struct RequireContextValue(FxIndexMap<RcStr, RcStr>);

#[turbo_tasks::value_impl]
impl RequireContextValue {
#[turbo_tasks::function]
pub async fn from_context_map(map: Vc<RequireContextMap>) -> Result<Vc<Self>> {
pub async fn from_context_map(map: Vc<RequireContextMap>) -> Result<Self> {
let mut context_map = FxIndexMap::default();

for (key, entry) in map.await?.iter() {
context_map.insert(key.clone(), entry.origin_relative.clone());
}

Ok(Vc::cell(context_map))
Ok(RequireContextValue(context_map))
}
}

Expand All @@ -3796,9 +3793,9 @@ pub enum WellKnownFunctionKind {
Require,
RequireResolve,
RequireContext,
RequireContextRequire(ResolvedVc<RequireContextValue>),
RequireContextRequireKeys(ResolvedVc<RequireContextValue>),
RequireContextRequireResolve(ResolvedVc<RequireContextValue>),
RequireContextRequire(RequireContextValue),
RequireContextRequireKeys(RequireContextValue),
RequireContextRequireResolve(RequireContextValue),
Define,
FsReadMethod(JsWord),
PathToFileUrl,
Expand Down Expand Up @@ -3845,7 +3842,7 @@ fn is_unresolved_id(i: &Id, unresolved_mark: Mark) -> bool {
#[doc(hidden)]
pub mod test_utils {
use anyhow::Result;
use turbo_tasks::{FxIndexMap, ResolvedVc, Vc};
use turbo_tasks::{FxIndexMap, Vc};
use turbopack_core::{compile_time_info::CompileTimeInfo, error::PrettyPrintError};

use super::{
Expand All @@ -3856,7 +3853,7 @@ pub mod test_utils {
analyzer::{
builtin::replace_builtin,
imports::{ImportAnnotations, ImportAttributes},
parse_require_context,
parse_require_context, RequireContextValue,
},
utils::module_value_to_well_known_object,
};
Expand Down Expand Up @@ -3907,7 +3904,7 @@ pub mod test_utils {
map.insert("./c".into(), format!("[context: {}]/c", options.dir).into());

JsValue::WellKnownFunction(WellKnownFunctionKind::RequireContextRequire(
ResolvedVc::cell(map),
RequireContextValue(map),
))
}
Err(err) => v.into_unknown(true, PrettyPrintError(&err).to_string()),
Expand Down
20 changes: 7 additions & 13 deletions turbopack/crates/turbopack-ecmascript/src/analyzer/well_known.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::mem::take;

use anyhow::Result;
use turbo_tasks::{ResolvedVc, Vc};
use turbo_tasks::Vc;
use turbopack_core::compile_time_info::CompileTimeInfo;
use url::Url;

Expand Down Expand Up @@ -354,10 +354,7 @@ pub fn require(args: Vec<JsValue>) -> JsValue {
}

/// (try to) statically evaluate `require.context(...)()`
async fn require_context_require(
val: ResolvedVc<RequireContextValue>,
args: Vec<JsValue>,
) -> Result<JsValue> {
async fn require_context_require(val: RequireContextValue, args: Vec<JsValue>) -> Result<JsValue> {
if args.is_empty() {
return Ok(JsValue::unknown(
JsValue::call(
Expand All @@ -384,8 +381,7 @@ async fn require_context_require(
));
};

let map = val.await?;
let Some(m) = map.get(s) else {
let Some(m) = val.0.get(s) else {
return Ok(JsValue::unknown(
JsValue::call(
Box::new(JsValue::WellKnownFunction(
Expand All @@ -407,12 +403,11 @@ async fn require_context_require(

/// (try to) statically evaluate `require.context(...).keys()`
async fn require_context_require_keys(
val: ResolvedVc<RequireContextValue>,
val: RequireContextValue,
args: Vec<JsValue>,
) -> Result<JsValue> {
Ok(if args.is_empty() {
let map = val.await?;
JsValue::array(map.keys().cloned().map(|k| k.into()).collect())
JsValue::array(val.0.keys().cloned().map(|k| k.into()).collect())
} else {
JsValue::unknown(
JsValue::call(
Expand All @@ -429,7 +424,7 @@ async fn require_context_require_keys(

/// (try to) statically evaluate `require.context(...).resolve()`
async fn require_context_require_resolve(
val: ResolvedVc<RequireContextValue>,
val: RequireContextValue,
args: Vec<JsValue>,
) -> Result<JsValue> {
if args.len() != 1 {
Expand Down Expand Up @@ -458,8 +453,7 @@ async fn require_context_require_resolve(
));
};

let map = val.await?;
let Some(m) = map.get(s) else {
let Some(m) = val.0.get(s) else {
return Ok(JsValue::unknown(
JsValue::call(
Box::new(JsValue::WellKnownFunction(
Expand Down
4 changes: 1 addition & 3 deletions turbopack/crates/turbopack-ecmascript/src/references/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2880,9 +2880,7 @@ async fn require_context_visitor(

Ok(JsValue::WellKnownFunction(
WellKnownFunctionKind::RequireContextRequire(
RequireContextValue::from_context_map(map)
.to_resolved()
.await?,
RequireContextValue::from_context_map(map).await?,
),
))
}
Expand Down

0 comments on commit 1bdc679

Please sign in to comment.