Skip to content

Commit 598750d

Browse files
sokrasokra
authored
very dynamic requests will only lead to a warning (#7640)
### Description they no longer lead to an additional module not found error ### Testing Instructions <!-- Give a quick description of steps to test your changes. --> Closes PACK-2673
1 parent 4b0415d commit 598750d

21 files changed

+352
-23
lines changed

‎crates/turbopack-ecmascript/src/lib.rs

+4
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,10 @@ pub struct EcmascriptOptions {
131131
/// External imports should used `__turbopack_import__` instead of
132132
/// `__turbopack_require__` and become async module references.
133133
pub import_externals: bool,
134+
/// Ignore very dynamic requests which doesn't have any static known part.
135+
/// If false, they will reference the whole directory. If true, they won't
136+
/// reference anything and lead to an runtime error instead.
137+
pub ignore_dynamic_requests: bool,
134138
}
135139

136140
#[turbo_tasks::value(serialization = "auto_for_input")]

‎crates/turbopack-ecmascript/src/references/dynamic_expression.rs

+71
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
use anyhow::Result;
2+
use serde::{Deserialize, Serialize};
3+
use swc_core::quote;
4+
use turbo_tasks::{debug::ValueDebugFormat, trace::TraceRawVcs, Vc};
5+
6+
use super::AstPath;
7+
use crate::{
8+
chunk::EcmascriptChunkingContext,
9+
code_gen::{CodeGenerateable, CodeGeneration},
10+
create_visitor,
11+
};
12+
13+
#[derive(PartialEq, Eq, TraceRawVcs, Serialize, Deserialize, ValueDebugFormat)]
14+
enum DynamicExpressionType {
15+
Promise,
16+
Normal,
17+
}
18+
19+
#[turbo_tasks::value]
20+
pub struct DynamicExpression {
21+
path: Vc<AstPath>,
22+
ty: DynamicExpressionType,
23+
}
24+
25+
#[turbo_tasks::value_impl]
26+
impl DynamicExpression {
27+
#[turbo_tasks::function]
28+
pub fn new(path: Vc<AstPath>) -> Vc<Self> {
29+
Self::cell(DynamicExpression {
30+
path,
31+
ty: DynamicExpressionType::Normal,
32+
})
33+
}
34+
35+
#[turbo_tasks::function]
36+
pub fn new_promise(path: Vc<AstPath>) -> Vc<Self> {
37+
Self::cell(DynamicExpression {
38+
path,
39+
ty: DynamicExpressionType::Promise,
40+
})
41+
}
42+
}
43+
44+
#[turbo_tasks::value_impl]
45+
impl CodeGenerateable for DynamicExpression {
46+
#[turbo_tasks::function]
47+
async fn code_generation(
48+
&self,
49+
_context: Vc<Box<dyn EcmascriptChunkingContext>>,
50+
) -> Result<Vc<CodeGeneration>> {
51+
let path = &self.path.await?;
52+
53+
let visitor = match self.ty {
54+
DynamicExpressionType::Normal => {
55+
create_visitor!(path, visit_mut_expr(expr: &mut Expr) {
56+
*expr = quote!("(() => { const e = new Error(\"Cannot find module as expression is too dynamic\"); e.code = 'MODULE_NOT_FOUND'; throw e; })()" as Expr);
57+
})
58+
}
59+
DynamicExpressionType::Promise => {
60+
create_visitor!(path, visit_mut_expr(expr: &mut Expr) {
61+
*expr = quote!("Promise.resolve().then(() => { const e = new Error(\"Cannot find module as expression is too dynamic\"); e.code = 'MODULE_NOT_FOUND'; throw e; })" as Expr);
62+
})
63+
}
64+
};
65+
66+
Ok(CodeGeneration {
67+
visitors: vec![visitor],
68+
}
69+
.cell())
70+
}
71+
}

‎crates/turbopack-ecmascript/src/references/mod.rs

+78-23
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ pub mod async_module;
33
pub mod cjs;
44
pub mod constant_condition;
55
pub mod constant_value;
6+
pub mod dynamic_expression;
67
pub mod esm;
78
pub mod node;
89
pub mod pattern_mapping;
@@ -123,6 +124,7 @@ use crate::{
123124
references::{
124125
async_module::{AsyncModule, OptionAsyncModule},
125126
cjs::{CjsRequireAssetReference, CjsRequireCacheAccess, CjsRequireResolveAssetReference},
127+
dynamic_expression::DynamicExpression,
126128
esm::{module_id::EsmModuleIdAssetReference, EsmBinding, UrlRewriteBehavior},
127129
node::PackageJsonReference,
128130
require_context::{RequireContextAssetReference, RequireContextMap},
@@ -337,6 +339,7 @@ struct AnalysisState<'a> {
337339
first_import_meta: bool,
338340
tree_shaking_mode: Option<TreeShakingMode>,
339341
import_externals: bool,
342+
ignore_dynamic_requests: bool,
340343
}
341344

342345
impl<'a> AnalysisState<'a> {
@@ -774,6 +777,7 @@ pub(crate) async fn analyse_ecmascript_module_internal(
774777
first_import_meta: true,
775778
tree_shaking_mode: options.tree_shaking_mode,
776779
import_externals: options.import_externals,
780+
ignore_dynamic_requests: options.ignore_dynamic_requests,
777781
};
778782

779783
enum Action {
@@ -1063,7 +1067,10 @@ pub(crate) async fn analyse_ecmascript_module_internal(
10631067
DiagnosticId::Lint(
10641068
errors::failed_to_analyse::ecmascript::NEW_URL_IMPORT_META.to_string(),
10651069
),
1066-
)
1070+
);
1071+
if options.ignore_dynamic_requests {
1072+
continue;
1073+
}
10671074
}
10681075
analysis.add_reference(UrlAssetReference::new(
10691076
origin,
@@ -1131,6 +1138,7 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
11311138
origin,
11321139
source,
11331140
compile_time_info,
1141+
ignore_dynamic_requests,
11341142
..
11351143
} = state;
11361144
fn explain_args(args: &[JsValue]) -> (String, String) {
@@ -1186,7 +1194,13 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
11861194
DiagnosticId::Lint(
11871195
errors::failed_to_analyse::ecmascript::DYNAMIC_IMPORT.to_string(),
11881196
),
1189-
)
1197+
);
1198+
if ignore_dynamic_requests {
1199+
analysis.add_code_gen(DynamicExpression::new_promise(Vc::cell(
1200+
ast_path.to_vec(),
1201+
)));
1202+
return Ok(());
1203+
}
11901204
}
11911205
analysis.add_reference(EsmAsyncAssetReference::new(
11921206
origin,
@@ -1219,7 +1233,11 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
12191233
DiagnosticId::Lint(
12201234
errors::failed_to_analyse::ecmascript::REQUIRE.to_string(),
12211235
),
1222-
)
1236+
);
1237+
if ignore_dynamic_requests {
1238+
analysis.add_code_gen(DynamicExpression::new(Vc::cell(ast_path.to_vec())));
1239+
return Ok(());
1240+
}
12231241
}
12241242
analysis.add_reference(CjsRequireAssetReference::new(
12251243
origin,
@@ -1262,7 +1280,11 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
12621280
DiagnosticId::Lint(
12631281
errors::failed_to_analyse::ecmascript::REQUIRE_RESOLVE.to_string(),
12641282
),
1265-
)
1283+
);
1284+
if ignore_dynamic_requests {
1285+
analysis.add_code_gen(DynamicExpression::new(Vc::cell(ast_path.to_vec())));
1286+
return Ok(());
1287+
}
12661288
}
12671289
analysis.add_reference(CjsRequireResolveAssetReference::new(
12681290
origin,
@@ -1327,7 +1349,10 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
13271349
DiagnosticId::Lint(
13281350
errors::failed_to_analyse::ecmascript::FS_METHOD.to_string(),
13291351
),
1330-
)
1352+
);
1353+
if ignore_dynamic_requests {
1354+
return Ok(());
1355+
}
13311356
}
13321357
analysis.add_reference(FileSourceReference::new(source, Pattern::new(pat)));
13331358
return Ok(());
@@ -1367,7 +1392,10 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
13671392
DiagnosticId::Lint(
13681393
errors::failed_to_analyse::ecmascript::PATH_METHOD.to_string(),
13691394
),
1370-
)
1395+
);
1396+
if ignore_dynamic_requests {
1397+
return Ok(());
1398+
}
13711399
}
13721400
analysis.add_reference(FileSourceReference::new(source, Pattern::new(pat)));
13731401
return Ok(());
@@ -1398,7 +1426,10 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
13981426
DiagnosticId::Lint(
13991427
errors::failed_to_analyse::ecmascript::PATH_METHOD.to_string(),
14001428
),
1401-
)
1429+
);
1430+
if ignore_dynamic_requests {
1431+
return Ok(());
1432+
}
14021433
}
14031434
analysis.add_reference(DirAssetReference::new(source, Pattern::new(pat)));
14041435
return Ok(());
@@ -1419,17 +1450,27 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
14191450
JsValue::member(Box::new(args[1].clone()), Box::new(0_f64.into()));
14201451
let first_arg = state.link_value(first_arg, in_try).await?;
14211452
let pat = js_value_to_pattern(&first_arg);
1422-
if !pat.has_constant_parts() {
1453+
let dynamic = !pat.has_constant_parts();
1454+
if dynamic {
14231455
show_dynamic_warning = true;
14241456
}
1425-
analysis.add_reference(CjsAssetReference::new(
1426-
origin,
1427-
Request::parse(Value::new(pat)),
1428-
issue_source(source, span),
1429-
in_try,
1430-
));
1457+
if !dynamic || !ignore_dynamic_requests {
1458+
analysis.add_reference(CjsAssetReference::new(
1459+
origin,
1460+
Request::parse(Value::new(pat)),
1461+
issue_source(source, span),
1462+
in_try,
1463+
));
1464+
}
1465+
}
1466+
let dynamic = !pat.has_constant_parts();
1467+
if dynamic {
1468+
show_dynamic_warning = true;
1469+
}
1470+
if !dynamic || !ignore_dynamic_requests {
1471+
analysis.add_reference(FileSourceReference::new(source, Pattern::new(pat)));
14311472
}
1432-
if show_dynamic_warning || !pat.has_constant_parts() {
1473+
if show_dynamic_warning {
14331474
let (args, hints) = explain_args(&args);
14341475
handler.span_warn_with_code(
14351476
span,
@@ -1439,7 +1480,6 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
14391480
),
14401481
);
14411482
}
1442-
analysis.add_reference(FileSourceReference::new(source, Pattern::new(pat)));
14431483
return Ok(());
14441484
}
14451485
let (args, hints) = explain_args(&args);
@@ -1465,6 +1505,9 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
14651505
errors::failed_to_analyse::ecmascript::CHILD_PROCESS_SPAWN.to_string(),
14661506
),
14671507
);
1508+
if ignore_dynamic_requests {
1509+
return Ok(());
1510+
}
14681511
}
14691512
analysis.add_reference(CjsAssetReference::new(
14701513
origin,
@@ -1499,6 +1542,7 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
14991542
errors::failed_to_analyse::ecmascript::NODE_PRE_GYP_FIND.to_string(),
15001543
),
15011544
);
1545+
// Always ignore this dynamic request
15021546
return Ok(());
15031547
}
15041548
analysis.add_reference(NodePreGypConfigReference::new(
@@ -1588,6 +1632,7 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
15881632
errors::failed_to_analyse::ecmascript::NODE_EXPRESS.to_string(),
15891633
),
15901634
);
1635+
// Always ignore this dynamic request
15911636
return Ok(());
15921637
}
15931638
match s {
@@ -2187,19 +2232,29 @@ async fn value_visitor_inner(
21872232
if *compile_time_info.environment().node_externals().await? {
21882233
// TODO check externals
21892234
match &**name {
2190-
"path" => JsValue::WellKnownObject(WellKnownObjectKind::PathModule),
2191-
"fs/promises" => JsValue::WellKnownObject(WellKnownObjectKind::FsModule),
2192-
"fs" => JsValue::WellKnownObject(WellKnownObjectKind::FsModule),
2193-
"child_process" => JsValue::WellKnownObject(WellKnownObjectKind::ChildProcess),
2194-
"os" => JsValue::WellKnownObject(WellKnownObjectKind::OsModule),
2195-
"process" => JsValue::WellKnownObject(WellKnownObjectKind::NodeProcess),
2235+
"node:path" | "path" => {
2236+
JsValue::WellKnownObject(WellKnownObjectKind::PathModule)
2237+
}
2238+
"node:fs/promises" | "fs/promises" => {
2239+
JsValue::WellKnownObject(WellKnownObjectKind::FsModule)
2240+
}
2241+
"node:fs" | "fs" => JsValue::WellKnownObject(WellKnownObjectKind::FsModule),
2242+
"node:child_process" | "child_process" => {
2243+
JsValue::WellKnownObject(WellKnownObjectKind::ChildProcess)
2244+
}
2245+
"node:os" | "os" => JsValue::WellKnownObject(WellKnownObjectKind::OsModule),
2246+
"node:process" | "process" => {
2247+
JsValue::WellKnownObject(WellKnownObjectKind::NodeProcess)
2248+
}
21962249
"@mapbox/node-pre-gyp" => {
21972250
JsValue::WellKnownObject(WellKnownObjectKind::NodePreGyp)
21982251
}
21992252
"node-gyp-build" => {
22002253
JsValue::WellKnownFunction(WellKnownFunctionKind::NodeGypBuild)
22012254
}
2202-
"bindings" => JsValue::WellKnownFunction(WellKnownFunctionKind::NodeBindings),
2255+
"node:bindings" | "bindings" => {
2256+
JsValue::WellKnownFunction(WellKnownFunctionKind::NodeBindings)
2257+
}
22032258
"express" => JsValue::WellKnownFunction(WellKnownFunctionKind::NodeExpress),
22042259
"strong-globalize" => {
22052260
JsValue::WellKnownFunction(WellKnownFunctionKind::NodeStrongGlobalize)

‎crates/turbopack-tests/tests/snapshot.rs

+1
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ async fn run_test(resource: String) -> Result<Vc<FileSystemPath>> {
261261
..Default::default()
262262
})),
263263
preset_env_versions: Some(env),
264+
ignore_dynamic_requests: true,
264265
rules: vec![(
265266
ContextCondition::InDirectory("node_modules".to_string()),
266267
ModuleOptionsContext {

‎crates/turbopack-tests/tests/snapshot/dynamic-request/very-dynamic/input/index.js

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import child_process from "node:child_process";
2+
import fs, { readFileSync } from "node:fs";
3+
4+
const unknown = Math.random();
5+
6+
child_process.spawnSync(unknown);
7+
child_process.spawnSync("node", unknown);
8+
child_process.spawnSync("node", [unknown, unknown]);
9+
10+
require(unknown);
11+
12+
import(unknown);
13+
14+
fs.readFileSync(unknown);
15+
readFileSync(unknown);
16+
17+
new URL(unknown, import.meta.url);

‎crates/turbopack-tests/tests/snapshot/dynamic-request/very-dynamic/issues/__l___lint TP1001__ import(FreeVar(Math)[__quo__ra-fcce98.txt

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
warning - [analysis] [project]/crates/turbopack-tests/tests/snapshot/dynamic-request/very-dynamic/input/index.js /crates/turbopack-tests/tests/snapshot/dynamic-request/very-dynamic/input/index.js:12:0 lint TP1001 import(FreeVar(Math)["random"]()) is very dynamic
2+
3+
8 | child_process.spawnSync("node", [unknown, unknown]);
4+
9 |
5+
10 | require(unknown);
6+
11 |
7+
+ v-------------v
8+
12 + import(unknown);
9+
+ ^-------------^
10+
13 |
11+
14 | fs.readFileSync(unknown);
12+
15 | readFileSync(unknown);
13+
16 |

‎crates/turbopack-tests/tests/snapshot/dynamic-request/very-dynamic/issues/__l___lint TP1002__ require(FreeVar(Math)[__quo__r-fa7bb4.txt

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
warning - [analysis] [project]/crates/turbopack-tests/tests/snapshot/dynamic-request/very-dynamic/input/index.js /crates/turbopack-tests/tests/snapshot/dynamic-request/very-dynamic/input/index.js:10:0 lint TP1002 require(FreeVar(Math)["random"]()) is very dynamic
2+
3+
6 | child_process.spawnSync(unknown);
4+
7 | child_process.spawnSync("node", unknown);
5+
8 | child_process.spawnSync("node", [unknown, unknown]);
6+
9 |
7+
+ v--------------v
8+
10 + require(unknown);
9+
+ ^--------------^
10+
11 |
11+
12 | import(unknown);
12+
13 |
13+
14 | fs.readFileSync(unknown);

‎crates/turbopack-tests/tests/snapshot/dynamic-request/very-dynamic/issues/__l___lint TP1004__ fs.readFileSync(FreeVar(Math)[-45dc7a.txt

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
warning - [analysis] [project]/crates/turbopack-tests/tests/snapshot/dynamic-request/very-dynamic/input/index.js /crates/turbopack-tests/tests/snapshot/dynamic-request/very-dynamic/input/index.js:15:0 lint TP1004 fs.readFileSync(FreeVar(Math)["random"]()) is very dynamic
2+
3+
11 |
4+
12 | import(unknown);
5+
13 |
6+
14 | fs.readFileSync(unknown);
7+
+ v-------------------v
8+
15 + readFileSync(unknown);
9+
+ ^-------------------^
10+
16 |
11+
17 | new URL(unknown, import.meta.url);
12+
18 |

0 commit comments

Comments
 (0)