Skip to content

Commit f607d0c

Browse files
authored
[turbopack] Remove the final uses of Value<..> and delete the type (#80144)
Remove the final uses of `Value<..>` and destroy it. ### Why? The `Value<>` type is confusing and error prone, lets destroy it! As discussed over slack, turbo_tasks::Value type always implements is_resolved as true and is_transient as false which is incorrect and can enable smuggling transient vcs into turbo tasks. • https://vercel.slack.com/archives/C03EWR7LGEN/p1743456121241529https://vercel.slack.com/archives/C04NGV98TPS/p1743455453764879
1 parent 1d70e57 commit f607d0c

File tree

21 files changed

+120
-181
lines changed

21 files changed

+120
-181
lines changed

crates/next-api/src/pages.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ use serde::{Deserialize, Serialize};
2929
use tracing::Instrument;
3030
use turbo_rcstr::{RcStr, rcstr};
3131
use turbo_tasks::{
32-
Completion, FxIndexMap, NonLocalValue, ResolvedVc, TaskInput, Value, ValueToString, Vc,
33-
fxindexmap, fxindexset, trace::TraceRawVcs,
32+
Completion, FxIndexMap, NonLocalValue, ResolvedVc, TaskInput, ValueToString, Vc, fxindexmap,
33+
fxindexset, trace::TraceRawVcs,
3434
};
3535
use turbo_tasks_fs::{
3636
self, File, FileContent, FileSystem, FileSystemPath, FileSystemPathOption, VirtualFileSystem,
@@ -635,7 +635,7 @@ impl PagesProject {
635635
NextMode::Build => rcstr!("next/dist/client/next-turbopack.js"),
636636
},
637637
)),
638-
Value::new(EcmaScriptModulesReferenceSubType::Undefined),
638+
EcmaScriptModulesReferenceSubType::Undefined,
639639
false,
640640
None,
641641
)

crates/next-core/src/app_segment_config.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ pub async fn parse_segment_config_from_source(
272272

273273
let result = &*parse(
274274
*source,
275-
turbo_tasks::Value::new(if path.path.ends_with(".ts") {
275+
if path.path.ends_with(".ts") {
276276
EcmascriptModuleAssetType::Typescript {
277277
tsx: false,
278278
analyze_types: false,
@@ -284,7 +284,7 @@ pub async fn parse_segment_config_from_source(
284284
}
285285
} else {
286286
EcmascriptModuleAssetType::Ecmascript
287-
}),
287+
},
288288
EcmascriptInputTransforms::empty(),
289289
)
290290
.await?;

turbopack/crates/turbo-tasks-testing/tests/all_in_one.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
use anyhow::{Result, bail};
66
use turbo_rcstr::{RcStr, rcstr};
7-
use turbo_tasks::{ResolvedVc, Value, ValueToString, Vc};
7+
use turbo_tasks::{ResolvedVc, TaskInput, ValueToString, Vc};
88
use turbo_tasks_testing::{Registration, register, run};
99

1010
static REGISTRATION: Registration = register!();
@@ -27,7 +27,7 @@ async fn all_in_one() {
2727
}
2828
.into();
2929

30-
let result = my_function(a, b.get_last(), c, Value::new(MyEnumValue::Yeah(42)));
30+
let result = my_function(a, b.get_last(), c, MyEnumValue::Yeah(42));
3131
assert_eq!(*result.my_trait_function().await?, "42");
3232
assert_eq!(*result.my_trait_function2().await?, "42");
3333
assert_eq!(*result.my_trait_function3().await?, "4242");
@@ -63,12 +63,12 @@ async fn all_in_one() {
6363
.unwrap()
6464
}
6565

66-
#[turbo_tasks::value(transparent, serialization = "auto_for_input")]
66+
#[turbo_tasks::value(transparent)]
6767
#[derive(Debug, Clone, Hash)]
6868
struct MyTransparentValue(u32);
6969

70-
#[turbo_tasks::value(shared, serialization = "auto_for_input")]
71-
#[derive(Debug, Clone, Hash)]
70+
#[turbo_tasks::value(shared)]
71+
#[derive(Debug, Clone, Hash, TaskInput)]
7272
enum MyEnumValue {
7373
Yeah(u32),
7474
Nah,
@@ -159,12 +159,12 @@ async fn my_function(
159159
a: Vc<MyTransparentValue>,
160160
b: Vc<MyEnumValue>,
161161
c: Vc<MyStructValue>,
162-
d: Value<MyEnumValue>,
162+
d: MyEnumValue,
163163
) -> Result<Vc<MyStructValue>> {
164164
assert_eq!(*a.await?, 4242);
165165
assert_eq!(*b.await?, MyEnumValue::Yeah(42));
166166
assert_eq!(c.await?.value, 42);
167-
assert_eq!(d.into_value(), MyEnumValue::Yeah(42));
167+
assert_eq!(d, MyEnumValue::Yeah(42));
168168
Ok(c)
169169
}
170170

turbopack/crates/turbo-tasks/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ pub use state::{State, TransientState};
123123
pub use task::{SharedReference, TypedSharedReference, task_input::TaskInput};
124124
pub use trait_ref::{IntoTraitRef, TraitRef};
125125
pub use turbo_tasks_macros::{TaskInput, function, value_impl};
126-
pub use value::{TransientInstance, TransientValue, Value};
126+
pub use value::{TransientInstance, TransientValue};
127127
pub use value_type::{TraitMethod, TraitType, ValueType};
128128
pub use vc::{
129129
Dynamic, NonLocalValue, OperationValue, OperationVc, OptionVcExt, ReadVcFuture, ResolvedVc,

turbopack/crates/turbo-tasks/src/marker_trait.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ macro_rules! impl_auto_marker_trait {
7878
{}
7979
unsafe impl<T: $trait> $trait for $crate::State<T> {}
8080
unsafe impl<T: $trait> $trait for $crate::TransientState<T> {}
81-
unsafe impl<T: $trait> $trait for $crate::Value<T> {}
8281
unsafe impl<T: $trait> $trait for $crate::TransientValue<T> {}
8382
unsafe impl<T: $trait> $trait for $crate::TransientInstance<T> {}
8483

turbopack/crates/turbo-tasks/src/task/task_input.rs

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use std::{
2-
any::Any, collections::BTreeMap, fmt::Debug, future::Future, hash::Hash, sync::Arc,
3-
time::Duration,
2+
collections::BTreeMap, fmt::Debug, future::Future, hash::Hash, sync::Arc, time::Duration,
43
};
54

65
use anyhow::Result;
@@ -9,7 +8,7 @@ use serde::{Deserialize, Serialize};
98
use turbo_rcstr::RcStr;
109

1110
use crate::{
12-
MagicAny, ResolvedVc, TaskId, TransientInstance, TransientValue, Value, ValueTypeId, Vc,
11+
MagicAny, ResolvedVc, TaskId, TransientInstance, TransientValue, ValueTypeId, Vc,
1312
trace::TraceRawVcs,
1413
};
1514

@@ -170,29 +169,6 @@ where
170169
}
171170
}
172171

173-
impl<T> TaskInput for Value<T>
174-
where
175-
T: Any
176-
+ std::fmt::Debug
177-
+ Clone
178-
+ std::hash::Hash
179-
+ Eq
180-
+ Send
181-
+ Sync
182-
+ Serialize
183-
+ for<'de> Deserialize<'de>
184-
+ TraceRawVcs
185-
+ 'static,
186-
{
187-
fn is_resolved(&self) -> bool {
188-
true
189-
}
190-
191-
fn is_transient(&self) -> bool {
192-
false
193-
}
194-
}
195-
196172
impl<T> TaskInput for TransientValue<T>
197173
where
198174
T: MagicAny + Clone + Debug + Hash + Eq + TraceRawVcs + 'static,

turbopack/crates/turbo-tasks/src/value.rs

Lines changed: 1 addition & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,68 +1,12 @@
11
use std::{fmt::Debug, marker::PhantomData, ops::Deref};
22

33
use anyhow::Result;
4-
use serde::{Deserialize, Serialize};
54

65
use crate::{
7-
ReadRef, SharedReference,
8-
debug::{ValueDebugFormat, ValueDebugString},
6+
SharedReference,
97
trace::{TraceRawVcs, TraceRawVcsContext},
108
};
119

12-
/// Pass a value by value (`Value<Xxx>`) instead of by reference (`Vc<Xxx>`).
13-
///
14-
/// Persistent, requires serialization.
15-
#[derive(Debug, PartialEq, Eq, Clone, Hash, Serialize, Deserialize)]
16-
pub struct Value<T> {
17-
inner: T,
18-
}
19-
20-
impl<T> Value<T> {
21-
pub fn new(value: T) -> Self {
22-
Self { inner: value }
23-
}
24-
25-
pub fn into_value(self) -> T {
26-
self.inner
27-
}
28-
}
29-
30-
impl<T> Deref for Value<T> {
31-
type Target = T;
32-
33-
fn deref(&self) -> &Self::Target {
34-
&self.inner
35-
}
36-
}
37-
38-
impl<T: Copy> Copy for Value<T> {}
39-
40-
impl<T: Default> Default for Value<T> {
41-
fn default() -> Self {
42-
Value::new(Default::default())
43-
}
44-
}
45-
46-
impl<T: ValueDebugFormat> Value<T> {
47-
pub async fn dbg(&self) -> Result<ReadRef<ValueDebugString>> {
48-
self.dbg_depth(usize::MAX).await
49-
}
50-
51-
pub async fn dbg_depth(&self, depth: usize) -> Result<ReadRef<ValueDebugString>> {
52-
self.inner
53-
.value_debug_format(depth)
54-
.try_to_value_debug_string()
55-
.await?
56-
.await
57-
}
58-
}
59-
60-
impl<T: TraceRawVcs> TraceRawVcs for Value<T> {
61-
fn trace_raw_vcs(&self, trace_context: &mut TraceRawVcsContext) {
62-
self.inner.trace_raw_vcs(trace_context)
63-
}
64-
}
65-
6610
/// Pass a value by value (`Value<Xxx>`) instead of by reference (`Vc<Xxx>`).
6711
///
6812
/// Doesn't require serialization, and won't be stored in the persistent cache

turbopack/crates/turbopack-browser/src/chunking_context.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize};
33
use tracing::Instrument;
44
use turbo_rcstr::{RcStr, rcstr};
55
use turbo_tasks::{
6-
NonLocalValue, ResolvedVc, TaskInput, TryJoinIterExt, Upcast, Value, ValueToString, Vc,
6+
NonLocalValue, ResolvedVc, TaskInput, TryJoinIterExt, Upcast, ValueToString, Vc,
77
trace::TraceRawVcs,
88
};
99
use turbo_tasks_fs::FileSystemPath;
@@ -38,7 +38,7 @@ use crate::ecmascript::{
3838
};
3939

4040
#[turbo_tasks::value]
41-
#[derive(Debug, Clone, Copy, Hash)]
41+
#[derive(Debug, Clone, Copy, Hash, TaskInput)]
4242
pub enum CurrentChunkMethod {
4343
StringLiteral,
4444
DocumentCurrentScript,
@@ -161,7 +161,7 @@ impl BrowserChunkingContextBuilder {
161161
}
162162

163163
pub fn build(self) -> Vc<BrowserChunkingContext> {
164-
BrowserChunkingContext::new(Value::new(self.chunking_context))
164+
BrowserChunkingContext::new(self.chunking_context)
165165
}
166166
}
167167

@@ -172,7 +172,7 @@ impl BrowserChunkingContextBuilder {
172172
/// It splits "node_modules" separately as these are less likely to change
173173
/// during development
174174
#[turbo_tasks::value(serialization = "auto_for_input")]
175-
#[derive(Debug, Clone, Hash)]
175+
#[derive(Debug, Clone, Hash, TaskInput)]
176176
pub struct BrowserChunkingContext {
177177
name: Option<RcStr>,
178178
/// The root path of the project
@@ -295,8 +295,8 @@ impl BrowserChunkingContext {
295295
#[turbo_tasks::value_impl]
296296
impl BrowserChunkingContext {
297297
#[turbo_tasks::function]
298-
fn new(this: Value<BrowserChunkingContext>) -> Vc<Self> {
299-
this.into_value().cell()
298+
fn new(this: BrowserChunkingContext) -> Vc<Self> {
299+
this.cell()
300300
}
301301

302302
#[turbo_tasks::function]
@@ -323,7 +323,7 @@ impl BrowserChunkingContext {
323323
ident: Vc<AssetIdent>,
324324
evaluatable_assets: Vc<EvaluatableAssets>,
325325
other_chunks: Vc<OutputAssets>,
326-
source: Value<EcmascriptDevChunkListSource>,
326+
source: EcmascriptDevChunkListSource,
327327
) -> Vc<Box<dyn OutputAsset>> {
328328
Vc::upcast(EcmascriptDevChunkList::new(
329329
self,
@@ -565,7 +565,7 @@ impl ChunkingContext for BrowserChunkingContext {
565565
ident,
566566
EvaluatableAssets::empty(),
567567
Vc::cell(assets.clone()),
568-
Value::new(EcmascriptDevChunkListSource::Dynamic),
568+
EcmascriptDevChunkListSource::Dynamic,
569569
)
570570
.to_resolved()
571571
.await?,
@@ -634,7 +634,7 @@ impl ChunkingContext for BrowserChunkingContext {
634634
ident,
635635
entries,
636636
other_assets,
637-
Value::new(EcmascriptDevChunkListSource::Entry),
637+
EcmascriptDevChunkListSource::Entry,
638638
)
639639
.to_resolved()
640640
.await?,

turbopack/crates/turbopack-browser/src/ecmascript/evaluate/chunk.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use either::Either;
55
use indoc::writedoc;
66
use serde::Serialize;
77
use turbo_rcstr::{RcStr, rcstr};
8-
use turbo_tasks::{ReadRef, ResolvedVc, TryJoinIterExt, Value, ValueToString, Vc};
8+
use turbo_tasks::{ReadRef, ResolvedVc, TryJoinIterExt, ValueToString, Vc};
99
use turbo_tasks_fs::{File, FileSystemPath, rope::RopeBuilder};
1010
use turbopack_core::{
1111
asset::{Asset, AssetContent},
@@ -171,7 +171,7 @@ impl EcmascriptBrowserEvaluateChunk {
171171
environment,
172172
chunking_context.chunk_base_path(),
173173
chunking_context.chunk_suffix_path(),
174-
Value::new(chunking_context.runtime_type()),
174+
chunking_context.runtime_type(),
175175
output_root_to_root_path,
176176
source_maps,
177177
);
@@ -182,7 +182,7 @@ impl EcmascriptBrowserEvaluateChunk {
182182
environment,
183183
chunking_context.chunk_base_path(),
184184
chunking_context.chunk_suffix_path(),
185-
Value::new(chunking_context.runtime_type()),
185+
chunking_context.runtime_type(),
186186
output_root_to_root_path,
187187
source_maps,
188188
);

turbopack/crates/turbopack-browser/src/ecmascript/list/asset.rs

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use anyhow::Result;
2+
use serde::{Deserialize, Serialize};
23
use turbo_rcstr::{RcStr, rcstr};
3-
use turbo_tasks::{ResolvedVc, Value, ValueToString, Vc};
4+
use turbo_tasks::{NonLocalValue, ResolvedVc, TaskInput, ValueToString, Vc, trace::TraceRawVcs};
45
use turbo_tasks_fs::FileSystemPath;
56
use turbopack_core::{
67
asset::{Asset, AssetContent},
@@ -42,14 +43,14 @@ impl EcmascriptDevChunkList {
4243
ident: ResolvedVc<AssetIdent>,
4344
evaluatable_assets: ResolvedVc<EvaluatableAssets>,
4445
chunks: ResolvedVc<OutputAssets>,
45-
source: Value<EcmascriptDevChunkListSource>,
46+
source: EcmascriptDevChunkListSource,
4647
) -> Vc<Self> {
4748
EcmascriptDevChunkList {
4849
chunking_context,
4950
ident,
5051
evaluatable_assets,
5152
chunks,
52-
source: source.into_value(),
53+
source,
5354
}
5455
.cell()
5556
}
@@ -112,8 +113,19 @@ impl Asset for EcmascriptDevChunkList {
112113
}
113114
}
114115

115-
#[derive(Debug, Clone, Copy, Hash)]
116-
#[turbo_tasks::value(serialization = "auto_for_input")]
116+
#[derive(
117+
Eq,
118+
PartialEq,
119+
Debug,
120+
Clone,
121+
Copy,
122+
Hash,
123+
TaskInput,
124+
NonLocalValue,
125+
TraceRawVcs,
126+
Serialize,
127+
Deserialize,
128+
)]
117129
#[serde(rename_all = "camelCase")]
118130
pub enum EcmascriptDevChunkListSource {
119131
/// The chunk list is from a runtime entry.

turbopack/crates/turbopack-core/src/compile_time_info.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use anyhow::Result;
22
use serde::{Deserialize, Serialize};
33
use turbo_rcstr::RcStr;
4-
use turbo_tasks::{FxIndexMap, NonLocalValue, ResolvedVc, Vc, trace::TraceRawVcs};
4+
use turbo_tasks::{FxIndexMap, NonLocalValue, ResolvedVc, TaskInput, Vc, trace::TraceRawVcs};
55
use turbo_tasks_fs::FileSystemPath;
66

77
use crate::environment::Environment;
@@ -101,8 +101,8 @@ macro_rules! free_var_references {
101101

102102
// TODO: replace with just a `serde_json::Value`
103103
// https://linear.app/vercel/issue/WEB-1641/compiletimedefinevalue-should-just-use-serde-jsonvalue
104-
#[turbo_tasks::value(serialization = "auto_for_input")]
105-
#[derive(Debug, Clone, Hash)]
104+
#[turbo_tasks::value]
105+
#[derive(Debug, Clone, Hash, TaskInput)]
106106
pub enum CompileTimeDefineValue {
107107
Bool(bool),
108108
String(RcStr),

0 commit comments

Comments
 (0)