Skip to content

Commit

Permalink
very dynamic requests will only lead to a warning
Browse files Browse the repository at this point in the history 
they no longer lead to an additional module not found error
  • Loading branch information
@sokra
sokra committed Mar 6, 2024
1 parent ce591fd commit c8c996d
Show file tree
Hide file tree
Showing 21 changed files with 352 additions and 23 deletions.
4 changes: 4 additions & 0 deletions crates/turbopack-ecmascript/src/lib.rs
View file
Expand Up @@ -131,6 +131,10 @@ pub struct EcmascriptOptions {
/// External imports should used `__turbopack_import__` instead of
/// `__turbopack_require__` and become async module references.
pub import_externals: bool,
/// Ignore very dynamic requests which doesn't have any static known part.
/// If false, they will reference the whole directory. If true, they won't
/// reference anything and lead to an runtime error instead.
pub ignore_dynamic_requests: bool,
}

#[turbo_tasks::value(serialization = "auto_for_input")]
Expand Down
71 changes: 71 additions & 0 deletions crates/turbopack-ecmascript/src/references/dynamic_expression.rs
View file
@@ -0,0 +1,71 @@
use anyhow::Result;
use serde::{Deserialize, Serialize};
use swc_core::quote;
use turbo_tasks::{debug::ValueDebugFormat, trace::TraceRawVcs, Vc};

use super::AstPath;
use crate::{
chunk::EcmascriptChunkingContext,
code_gen::{CodeGenerateable, CodeGeneration},
create_visitor,
};

#[derive(PartialEq, Eq, TraceRawVcs, Serialize, Deserialize, ValueDebugFormat)]
enum DynamicExpressionType {
Promise,
Normal,
}

#[turbo_tasks::value]
pub struct DynamicExpression {
path: Vc<AstPath>,
ty: DynamicExpressionType,
}

#[turbo_tasks::value_impl]
impl DynamicExpression {
#[turbo_tasks::function]
pub fn new(path: Vc<AstPath>) -> Vc<Self> {
Self::cell(DynamicExpression {
path,
ty: DynamicExpressionType::Normal,
})
}

#[turbo_tasks::function]
pub fn new_promise(path: Vc<AstPath>) -> Vc<Self> {
Self::cell(DynamicExpression {
path,
ty: DynamicExpressionType::Promise,
})
}
}

#[turbo_tasks::value_impl]
impl CodeGenerateable for DynamicExpression {
#[turbo_tasks::function]
async fn code_generation(
&self,
_context: Vc<Box<dyn EcmascriptChunkingContext>>,
) -> Result<Vc<CodeGeneration>> {
let path = &self.path.await?;

let visitor = match self.ty {
DynamicExpressionType::Normal => {
create_visitor!(path, visit_mut_expr(expr: &mut Expr) {
*expr = quote!("(() => { const e = new Error(\"Cannot find module as expression is too dynamic\"); e.code = 'MODULE_NOT_FOUND'; throw e; })()" as Expr);
})
}
DynamicExpressionType::Promise => {
create_visitor!(path, visit_mut_expr(expr: &mut Expr) {
*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);
})
}
};

Ok(CodeGeneration {
visitors: vec![visitor],
}
.cell())
}
}
101 changes: 78 additions & 23 deletions crates/turbopack-ecmascript/src/references/mod.rs
View file
Expand Up @@ -3,6 +3,7 @@ pub mod async_module;
pub mod cjs;
pub mod constant_condition;
pub mod constant_value;
pub mod dynamic_expression;
pub mod esm;
pub mod node;
pub mod pattern_mapping;
Expand Down Expand Up @@ -123,6 +124,7 @@ use crate::{
references::{
async_module::{AsyncModule, OptionAsyncModule},
cjs::{CjsRequireAssetReference, CjsRequireCacheAccess, CjsRequireResolveAssetReference},
dynamic_expression::DynamicExpression,
esm::{module_id::EsmModuleIdAssetReference, EsmBinding, UrlRewriteBehavior},
node::PackageJsonReference,
require_context::{RequireContextAssetReference, RequireContextMap},
Expand Down Expand Up @@ -337,6 +339,7 @@ struct AnalysisState<'a> {
first_import_meta: bool,
tree_shaking_mode: Option<TreeShakingMode>,
import_externals: bool,
ignore_dynamic_requests: bool,
}

impl<'a> AnalysisState<'a> {
Expand Down Expand Up @@ -774,6 +777,7 @@ pub(crate) async fn analyse_ecmascript_module_internal(
first_import_meta: true,
tree_shaking_mode: options.tree_shaking_mode,
import_externals: options.import_externals,
ignore_dynamic_requests: options.ignore_dynamic_requests,
};

enum Action {
Expand Down Expand Up @@ -1063,7 +1067,10 @@ pub(crate) async fn analyse_ecmascript_module_internal(
DiagnosticId::Lint(
errors::failed_to_analyse::ecmascript::NEW_URL_IMPORT_META.to_string(),
),
)
);
if options.ignore_dynamic_requests {
continue;
}
}
analysis.add_reference(UrlAssetReference::new(
origin,
Expand Down Expand Up @@ -1131,6 +1138,7 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
origin,
source,
compile_time_info,
ignore_dynamic_requests,
..
} = state;
fn explain_args(args: &[JsValue]) -> (String, String) {
Expand Down Expand Up @@ -1186,7 +1194,13 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
DiagnosticId::Lint(
errors::failed_to_analyse::ecmascript::DYNAMIC_IMPORT.to_string(),
),
)
);
if ignore_dynamic_requests {
analysis.add_code_gen(DynamicExpression::new_promise(Vc::cell(
ast_path.to_vec(),
)));
return Ok(());
}
}
analysis.add_reference(EsmAsyncAssetReference::new(
origin,
Expand Down Expand Up @@ -1219,7 +1233,11 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
DiagnosticId::Lint(
errors::failed_to_analyse::ecmascript::REQUIRE.to_string(),
),
)
);
if ignore_dynamic_requests {
analysis.add_code_gen(DynamicExpression::new(Vc::cell(ast_path.to_vec())));
return Ok(());
}
}
analysis.add_reference(CjsRequireAssetReference::new(
origin,
Expand Down Expand Up @@ -1262,7 +1280,11 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
DiagnosticId::Lint(
errors::failed_to_analyse::ecmascript::REQUIRE_RESOLVE.to_string(),
),
)
);
if ignore_dynamic_requests {
analysis.add_code_gen(DynamicExpression::new(Vc::cell(ast_path.to_vec())));
return Ok(());
}
}
analysis.add_reference(CjsRequireResolveAssetReference::new(
origin,
Expand Down Expand Up @@ -1327,7 +1349,10 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
DiagnosticId::Lint(
errors::failed_to_analyse::ecmascript::FS_METHOD.to_string(),
),
)
);
if ignore_dynamic_requests {
return Ok(());
}
}
analysis.add_reference(FileSourceReference::new(source, Pattern::new(pat)));
return Ok(());
Expand Down Expand Up @@ -1367,7 +1392,10 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
DiagnosticId::Lint(
errors::failed_to_analyse::ecmascript::PATH_METHOD.to_string(),
),
)
);
if ignore_dynamic_requests {
return Ok(());
}
}
analysis.add_reference(FileSourceReference::new(source, Pattern::new(pat)));
return Ok(());
Expand Down Expand Up @@ -1398,7 +1426,10 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
DiagnosticId::Lint(
errors::failed_to_analyse::ecmascript::PATH_METHOD.to_string(),
),
)
);
if ignore_dynamic_requests {
return Ok(());
}
}
analysis.add_reference(DirAssetReference::new(source, Pattern::new(pat)));
return Ok(());
Expand All @@ -1419,17 +1450,27 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
JsValue::member(Box::new(args[1].clone()), Box::new(0_f64.into()));
let first_arg = state.link_value(first_arg, in_try).await?;
let pat = js_value_to_pattern(&first_arg);
if !pat.has_constant_parts() {
let dynamic = !pat.has_constant_parts();
if dynamic {
show_dynamic_warning = true;
}
analysis.add_reference(CjsAssetReference::new(
origin,
Request::parse(Value::new(pat)),
issue_source(source, span),
in_try,
));
if !dynamic || !ignore_dynamic_requests {
analysis.add_reference(CjsAssetReference::new(
origin,
Request::parse(Value::new(pat)),
issue_source(source, span),
in_try,
));
}
}
let dynamic = !pat.has_constant_parts();
if dynamic {
show_dynamic_warning = true;
}
if !dynamic || !ignore_dynamic_requests {
analysis.add_reference(FileSourceReference::new(source, Pattern::new(pat)));
}
if show_dynamic_warning || !pat.has_constant_parts() {
if show_dynamic_warning {
let (args, hints) = explain_args(&args);
handler.span_warn_with_code(
span,
Expand All @@ -1439,7 +1480,6 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
),
);
}
analysis.add_reference(FileSourceReference::new(source, Pattern::new(pat)));
return Ok(());
}
let (args, hints) = explain_args(&args);
Expand All @@ -1465,6 +1505,9 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
errors::failed_to_analyse::ecmascript::CHILD_PROCESS_SPAWN.to_string(),
),
);
if ignore_dynamic_requests {
return Ok(());
}
}
analysis.add_reference(CjsAssetReference::new(
origin,
Expand Down Expand Up @@ -1499,6 +1542,7 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
errors::failed_to_analyse::ecmascript::NODE_PRE_GYP_FIND.to_string(),
),
);
// Always ignore this dynamic request
return Ok(());
}
analysis.add_reference(NodePreGypConfigReference::new(
Expand Down Expand Up @@ -1588,6 +1632,7 @@ async fn handle_call<G: Fn(Vec<Effect>) + Send + Sync>(
errors::failed_to_analyse::ecmascript::NODE_EXPRESS.to_string(),
),
);
// Always ignore this dynamic request
return Ok(());
}
match s {
Expand Down Expand Up @@ -2187,19 +2232,29 @@ async fn value_visitor_inner(
if *compile_time_info.environment().node_externals().await? {
// TODO check externals
match &**name {
"path" => JsValue::WellKnownObject(WellKnownObjectKind::PathModule),
"fs/promises" => JsValue::WellKnownObject(WellKnownObjectKind::FsModule),
"fs" => JsValue::WellKnownObject(WellKnownObjectKind::FsModule),
"child_process" => JsValue::WellKnownObject(WellKnownObjectKind::ChildProcess),
"os" => JsValue::WellKnownObject(WellKnownObjectKind::OsModule),
"process" => JsValue::WellKnownObject(WellKnownObjectKind::NodeProcess),
"node:path" | "path" => {
JsValue::WellKnownObject(WellKnownObjectKind::PathModule)
}
"node:fs/promises" | "fs/promises" => {
JsValue::WellKnownObject(WellKnownObjectKind::FsModule)
}
"node:fs" | "fs" => JsValue::WellKnownObject(WellKnownObjectKind::FsModule),
"node:child_process" | "child_process" => {
JsValue::WellKnownObject(WellKnownObjectKind::ChildProcess)
}
"node:os" | "os" => JsValue::WellKnownObject(WellKnownObjectKind::OsModule),
"node:process" | "process" => {
JsValue::WellKnownObject(WellKnownObjectKind::NodeProcess)
}
"@mapbox/node-pre-gyp" => {
JsValue::WellKnownObject(WellKnownObjectKind::NodePreGyp)
}
"node-gyp-build" => {
JsValue::WellKnownFunction(WellKnownFunctionKind::NodeGypBuild)
}
"bindings" => JsValue::WellKnownFunction(WellKnownFunctionKind::NodeBindings),
"node:bindings" | "bindings" => {
JsValue::WellKnownFunction(WellKnownFunctionKind::NodeBindings)
}
"express" => JsValue::WellKnownFunction(WellKnownFunctionKind::NodeExpress),
"strong-globalize" => {
JsValue::WellKnownFunction(WellKnownFunctionKind::NodeStrongGlobalize)
Expand Down
1 change: 1 addition & 0 deletions crates/turbopack-tests/tests/snapshot.rs
View file
Expand Up @@ -261,6 +261,7 @@ async fn run_test(resource: String) -> Result<Vc<FileSystemPath>> {
..Default::default()
})),
preset_env_versions: Some(env),
ignore_dynamic_requests: true,
rules: vec![(
ContextCondition::InDirectory("node_modules".to_string()),
ModuleOptionsContext {
Expand Down
17 changes: 17 additions & 0 deletions crates/turbopack-tests/tests/snapshot/dynamic-request/very-dynamic/input/index.js
View file
@@ -0,0 +1,17 @@
import child_process from "node:child_process";
import fs, { readFileSync } from "node:fs";

const unknown = Math.random();

child_process.spawnSync(unknown);
child_process.spawnSync("node", unknown);
child_process.spawnSync("node", [unknown, unknown]);

require(unknown);

import(unknown);

fs.readFileSync(unknown);
readFileSync(unknown);

new URL(unknown, import.meta.url);
13 changes: 13 additions & 0 deletions ...request/very-dynamic/issues/__l___lint TP1001__ import(FreeVar(Math)[__quo__ra-fcce98.txt
View file
@@ -0,0 +1,13 @@
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

8 | child_process.spawnSync("node", [unknown, unknown]);
9 |
10 | require(unknown);
11 |
+ v-------------v
12 + import(unknown);
+ ^-------------^
13 |
14 | fs.readFileSync(unknown);
15 | readFileSync(unknown);
16 |
13 changes: 13 additions & 0 deletions ...request/very-dynamic/issues/__l___lint TP1002__ require(FreeVar(Math)[__quo__r-fa7bb4.txt
View file
@@ -0,0 +1,13 @@
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

6 | child_process.spawnSync(unknown);
7 | child_process.spawnSync("node", unknown);
8 | child_process.spawnSync("node", [unknown, unknown]);
9 |
+ v--------------v
10 + require(unknown);
+ ^--------------^
11 |
12 | import(unknown);
13 |
14 | fs.readFileSync(unknown);
12 changes: 12 additions & 0 deletions ...request/very-dynamic/issues/__l___lint TP1004__ fs.readFileSync(FreeVar(Math)[-45dc7a.txt
View file
@@ -0,0 +1,12 @@
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

11 |
12 | import(unknown);
13 |
14 | fs.readFileSync(unknown);
+ v-------------------v
15 + readFileSync(unknown);
+ ^-------------------^
16 |
17 | new URL(unknown, import.meta.url);
18 |

0 comments on commit c8c996d

Please sign in to comment.