Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions crates/next-core/src/next_client/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use turbo_tasks_fs::FileSystemPath;
use turbopack::{
css::chunk::CssChunkType,
module_options::{
CssOptionsContext, EcmascriptOptionsContext, JsxTransformOptions, ModuleRule, TypeofWindow,
CssOptionsContext, EcmascriptOptionsContext, JsxTransformOptions, ModuleRule,
TypescriptTransformOptions, module_options_context::ModuleOptionsContext,
},
resolve_options_context::ResolveOptionsContext,
Expand All @@ -28,7 +28,7 @@ use turbopack_core::{
module_graph::export_usage::OptionExportUsageInfo,
resolve::{parse::Request, pattern::Pattern},
};
use turbopack_ecmascript::chunk::EcmascriptChunkType;
use turbopack_ecmascript::{TypeofWindow, chunk::EcmascriptChunkType};
use turbopack_node::{
execution_context::ExecutionContext,
transforms::postcss::{PostCssConfigLocation, PostCssTransformOptions},
Expand Down
6 changes: 4 additions & 2 deletions crates/next-core/src/next_server/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use turbopack::{
css::chunk::CssChunkType,
module_options::{
CssOptionsContext, EcmascriptOptionsContext, ExternalsTracingOptions, JsxTransformOptions,
ModuleOptionsContext, ModuleRule, TypeofWindow, TypescriptTransformOptions,
ModuleOptionsContext, ModuleRule, TypescriptTransformOptions,
},
resolve_options_context::ResolveOptionsContext,
transition::Transition,
Expand All @@ -28,7 +28,9 @@ use turbopack_core::{
module_graph::export_usage::OptionExportUsageInfo,
target::CompileTarget,
};
use turbopack_ecmascript::{chunk::EcmascriptChunkType, references::esm::UrlRewriteBehavior};
use turbopack_ecmascript::{
TypeofWindow, chunk::EcmascriptChunkType, references::esm::UrlRewriteBehavior,
};
use turbopack_ecmascript_plugins::transform::directives::{
client::ClientDirectiveTransformer, client_disallowed::ClientDisallowedDirectiveTransformer,
};
Expand Down
26 changes: 25 additions & 1 deletion turbopack/crates/turbopack-ecmascript/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,27 @@ pub enum TreeShakingMode {
#[turbo_tasks::value(transparent)]
pub struct OptionTreeShaking(pub Option<TreeShakingMode>);

/// The constant to replace `typeof window` with.
#[derive(
Copy,
Clone,
PartialEq,
Eq,
Debug,
Hash,
Serialize,
Deserialize,
TraceRawVcs,
NonLocalValue,
TaskInput,
)]
pub enum TypeofWindow {
Object,
Undefined,
}

#[turbo_tasks::value(shared)]
#[derive(Hash, Debug, Default, Copy, Clone)]
#[derive(Debug, Default, Copy, Clone)]
pub struct EcmascriptOptions {
/// variant of tree shaking to use
pub tree_shaking_mode: Option<TreeShakingMode>,
Expand All @@ -203,6 +222,11 @@ pub struct EcmascriptOptions {
/// Whether the modules in this context are never chunked/codegen-ed, but only used for
/// tracing.
pub is_tracing: bool,
// TODO this should just be handled via CompileTimeInfo FreeVarReferences, but then it
// (currently) wouldn't be possible to have different replacement values in user code vs
// node_modules.
/// Whether to replace `typeof window` with some constant value.
pub enable_typeof_window_inlining: Option<TypeofWindow>,
}

#[turbo_tasks::value]
Expand Down
46 changes: 37 additions & 9 deletions turbopack/crates/turbopack-ecmascript/src/references/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ use turbo_tasks::{
use turbo_tasks_fs::FileSystemPath;
use turbopack_core::{
compile_time_info::{
CompileTimeDefineValue, CompileTimeInfo, DefinableNameSegment, FreeVarReference,
FreeVarReferences, FreeVarReferencesIndividual, InputRelativeConstant,
CompileTimeDefineValue, CompileTimeDefines, CompileTimeInfo, DefinableNameSegment,
FreeVarReference, FreeVarReferences, FreeVarReferencesIndividual, InputRelativeConstant,
},
environment::Rendering,
error::PrettyPrintError,
Expand Down Expand Up @@ -130,7 +130,7 @@ use super::{
pub use crate::references::esm::export::{FollowExportsResult, follow_reexports};
use crate::{
EcmascriptInputTransforms, EcmascriptModuleAsset, EcmascriptParsable, SpecifiedModuleType,
TreeShakingMode,
TreeShakingMode, TypeofWindow,
analyzer::{
ConstantNumber, ConstantString, JsValueUrlKind, RequireContextValue,
builtin::early_replace_builtin,
Expand Down Expand Up @@ -617,10 +617,13 @@ pub async fn analyse_ecmascript_module_internal(
analysis.set_has_side_effect_free_directive(has_side_effect_free_directive);

let is_esm = eval_context.is_esm(specified_type);
let compile_time_info =
compile_time_info_for_module_type(*raw_module.compile_time_info, is_esm)
.to_resolved()
.await?;
let compile_time_info = compile_time_info_for_module_options(
*raw_module.compile_time_info,
is_esm,
options.enable_typeof_window_inlining,
)
.to_resolved()
.await?;

let pos = program.span().lo;
if analyze_types {
Expand Down Expand Up @@ -1471,9 +1474,10 @@ pub async fn analyse_ecmascript_module_internal(
}

#[turbo_tasks::function]
async fn compile_time_info_for_module_type(
async fn compile_time_info_for_module_options(
compile_time_info: Vc<CompileTimeInfo>,
is_esm: bool,
enable_typeof_window_inlining: Option<TypeofWindow>,
) -> Result<Vc<CompileTimeInfo>> {
let compile_time_info = compile_time_info.await?;
let free_var_references = compile_time_info.free_var_references;
Expand Down Expand Up @@ -1584,9 +1588,33 @@ async fn compile_time_info_for_module_type(
} else {
rcstr!("object").into()
});

let mut defines = compile_time_info.defines;
if let Some(enable_typeof_window_inlining) = enable_typeof_window_inlining {
let value = match enable_typeof_window_inlining {
TypeofWindow::Object => rcstr!("object"),
TypeofWindow::Undefined => rcstr!("undefined"),
};
let window = rcstr!("window");
let mut defines_value = defines.owned().await?;
defines_value
Copy link
Contributor

Choose a reason for hiding this comment

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

This code violates the Turbo Tasks Value Cells rule. The .owned() method should not be called on Vc types. To read from a Value Cell, defines should be awaited directly using defines.await?, which returns a ReadRef<T> and properly registers the current task as dependent on the cell. Replace defines.owned().await? with defines.await? to correctly read the cell's contents.

Suggested change
defines_value
defines.await?

Spotted by Diamond (based on custom rule: Turbo Tasks: Value Cells)

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

.entry(vec![
DefinableNameSegment::Name(window.clone()),
DefinableNameSegment::TypeOf,
])
.or_insert(value.clone().into());
free_var_references
.entry(vec![
DefinableNameSegment::Name(window),
DefinableNameSegment::TypeOf,
])
.or_insert(value.into());
defines = CompileTimeDefines(defines_value).resolved_cell()
}

Ok(CompileTimeInfo {
environment: compile_time_info.environment,
defines: compile_time_info.defines,
defines,
free_var_references: FreeVarReferences(free_var_references).resolved_cell(),
}
.cell())
Expand Down
21 changes: 0 additions & 21 deletions turbopack/crates/turbopack-ecmascript/src/transform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use std::{fmt::Debug, hash::Hash, sync::Arc};

use anyhow::Result;
use async_trait::async_trait;
use rustc_hash::FxHashMap;
use swc_core::{
atoms::{Atom, atom},
base::SwcComments,
Expand All @@ -15,7 +14,6 @@ use swc_core::{
assumptions::Assumptions,
helpers::{HELPERS, HelperData, Helpers},
},
optimization::inline_globals,
react::react,
},
utils::IsDirective,
Expand Down Expand Up @@ -44,9 +42,6 @@ pub enum EcmascriptInputTransform {
// swc.jsc.transform.react.runtime,
runtime: ResolvedVc<Option<RcStr>>,
},
GlobalTypeofs {
window_value: RcStr,
},
// These options are subset of swc_core::ecma::transforms::typescript::Config, but
// it doesn't derive `Copy` so repeating values in here
TypeScript {
Expand Down Expand Up @@ -139,22 +134,6 @@ impl EcmascriptInputTransform {
} = ctx;

Ok(match self {
EcmascriptInputTransform::GlobalTypeofs { window_value } => {
let mut typeofs: FxHashMap<Atom, Atom> = Default::default();
typeofs.insert(Atom::from("window"), Atom::from(&**window_value));

apply_transform(
program,
helpers,
inline_globals(
unresolved_mark,
Default::default(),
Default::default(),
Default::default(),
Arc::new(typeofs),
),
)
}
EcmascriptInputTransform::React {
development,
refresh,
Expand Down
12 changes: 2 additions & 10 deletions turbopack/crates/turbopack/src/module_options/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ impl ModuleOptions {
ignore_dynamic_requests,
import_externals,
esm_url_rewrite_behavior,
ref enable_typeof_window_inlining,
enable_typeof_window_inlining,
source_maps: ecmascript_source_maps,
..
},
Expand Down Expand Up @@ -282,6 +282,7 @@ impl ModuleOptions {
extract_source_map: matches!(ecmascript_source_maps, SourceMapsType::Full),
keep_last_successful_parse,
is_tracing,
enable_typeof_window_inlining,
..Default::default()
};
let ecmascript_options_vc = ecmascript_options.resolved_cell();
Expand All @@ -290,15 +291,6 @@ impl ModuleOptions {
postprocess.push(EcmascriptInputTransform::PresetEnv(environment));
}

if let Some(enable_typeof_window_inlining) = enable_typeof_window_inlining {
postprocess.push(EcmascriptInputTransform::GlobalTypeofs {
window_value: match enable_typeof_window_inlining {
TypeofWindow::Object => rcstr!("object"),
TypeofWindow::Undefined => rcstr!("undefined"),
},
});
}

let ts_transform = if let Some(options) = enable_typescript_transform {
let options = options.await?;
Some(EcmascriptInputTransform::TypeScript {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use turbopack_core::{
chunk::SourceMapsType, compile_time_info::CompileTimeInfo, condition::ContextCondition,
environment::Environment, resolve::options::ImportMapping,
};
use turbopack_ecmascript::{TreeShakingMode, references::esm::UrlRewriteBehavior};
use turbopack_ecmascript::{TreeShakingMode, TypeofWindow, references::esm::UrlRewriteBehavior};
pub use turbopack_mdx::MdxTransformOptions;
use turbopack_node::{
execution_context::ExecutionContext,
Expand Down Expand Up @@ -108,13 +108,6 @@ pub enum DecoratorsKind {
Ecma,
}

/// The types when replacing `typeof window` with a constant.
#[derive(Copy, Clone, PartialEq, Eq, Debug, TraceRawVcs, Serialize, Deserialize, NonLocalValue)]
pub enum TypeofWindow {
Object,
Undefined,
}

/// Configuration options for the decorators transform.
///
/// This is not part of Typescript transform: while there are typescript
Expand Down Expand Up @@ -223,6 +216,9 @@ pub struct ModuleOptionsContext {
#[derive(Clone, Default)]
#[serde(default)]
pub struct EcmascriptOptionsContext {
// TODO this should just be handled via CompileTimeInfo FreeVarReferences, but then it
// (currently) wouldn't be possible to have different replacement values in user code vs
// node_modules.
pub enable_typeof_window_inlining: Option<TypeofWindow>,
pub enable_jsx: Option<ResolvedVc<JsxTransformOptions>>,
/// Follow type references and resolve declaration files in additional to
Expand Down
Loading