Cleanup minify (vercel/turborepo#5696)

### Description * Pass MinifyType as value instead of Vc * Avoid panic in parsing code * add into_trait_ref_strongly_consistent_untracked method on trait vcs
vercel · Aug 9, 2023 · 01c9d6d · 01c9d6d
1 parent d7b5f37
commit 01c9d6d
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 13 deletions.
diff --git a/crates/turbo-tasks/src/raw_vc.rs b/crates/turbo-tasks/src/raw_vc.rs
@@ -102,6 +102,14 @@ impl RawVc {
         ReadRawVcFuture::new_untracked(self)
     }
 
+    /// INVALIDATION: Be careful with this, it will not track dependencies, so
+    /// using it could break cache invalidation.
+    pub(crate) fn into_strongly_consistent_trait_read_untracked<T: VcValueTrait + ?Sized>(
+        self,
+    ) -> ReadRawVcFuture<T, VcValueTraitCast<T>> {
+        ReadRawVcFuture::new_strongly_consistent_untracked(self)
+    }
+
     pub(crate) async fn resolve_trait(
         self,
         trait_type: TraitTypeId,
@@ -325,6 +333,19 @@ impl<T: ?Sized, Cast: VcCast> ReadRawVcFuture<T, Cast> {
             _cast: PhantomData,
         }
     }
+
+    fn new_strongly_consistent_untracked(vc: RawVc) -> Self {
+        let tt = turbo_tasks();
+        ReadRawVcFuture {
+            turbo_tasks: tt,
+            strongly_consistent: true,
+            current: vc,
+            untracked: true,
+            listener: None,
+            phantom_data: PhantomData,
+            _cast: PhantomData,
+        }
+    }
 }
 
 impl<T: ?Sized, Cast: VcCast> Future for ReadRawVcFuture<T, Cast> {

diff --git a/crates/turbo-tasks/src/trait_ref.rs b/crates/turbo-tasks/src/trait_ref.rs
@@ -131,6 +131,7 @@ pub trait IntoTraitRef {
 
     fn into_trait_ref(self) -> Self::Future;
     fn into_trait_ref_untracked(self) -> Self::Future;
+    fn into_trait_ref_strongly_consistent_untracked(self) -> Self::Future;
 }
 
 impl<T> IntoTraitRef for Vc<T>
@@ -148,4 +149,9 @@ where
     fn into_trait_ref_untracked(self) -> Self::Future {
         self.node.into_trait_read_untracked::<T>()
     }
+
+    fn into_trait_ref_strongly_consistent_untracked(self) -> Self::Future {
+        self.node
+            .into_strongly_consistent_trait_read_untracked::<T>()
+    }
 }
diff --git a/crates/turbopack-build/src/chunking_context.rs b/crates/turbopack-build/src/chunking_context.rs
@@ -1,8 +1,10 @@
 use anyhow::{bail, Result};
 use indexmap::IndexSet;
+use serde::{Deserialize, Serialize};
 use turbo_tasks::{
     graph::{AdjacencyMap, GraphTraversal},
-    TryJoinIterExt, Value, Vc,
+    trace::TraceRawVcs,
+    TaskInput, TryJoinIterExt, Value, Vc,
 };
 use turbo_tasks_fs::FileSystemPath;
 use turbopack_core::{
@@ -22,8 +24,21 @@ use crate::ecmascript::node::{
     chunk::EcmascriptBuildNodeChunk, entry::chunk::EcmascriptBuildNodeEntryChunk,
 };
 
-#[derive(Debug, Default)]
-#[turbo_tasks::value(shared)]
+#[derive(
+    Debug,
+    Default,
+    TaskInput,
+    Clone,
+    Copy,
+    PartialEq,
+    Eq,
+    PartialOrd,
+    Ord,
+    Hash,
+    Serialize,
+    Deserialize,
+    TraceRawVcs,
+)]
 pub enum MinifyType {
     #[default]
     Minify,
@@ -36,7 +51,7 @@ pub struct BuildChunkingContextBuilder {
 }
 
 impl BuildChunkingContextBuilder {
-    pub fn minify_type(mut self, minify_type: Vc<MinifyType>) -> Self {
+    pub fn minify_type(mut self, minify_type: MinifyType) -> Self {
         self.chunking_context.minify_type = minify_type;
         self
     }
@@ -77,7 +92,7 @@ pub struct BuildChunkingContext {
     /// The kind of runtime to include in the output.
     runtime_type: RuntimeType,
     /// Whether to minify resulting chunks
-    minify_type: Vc<MinifyType>,
+    minify_type: MinifyType,
 }
 
 impl BuildChunkingContext {
@@ -98,7 +113,7 @@ impl BuildChunkingContext {
                 layer: None,
                 environment,
                 runtime_type: Default::default(),
-                minify_type: MinifyType::Minify.cell(),
+                minify_type: MinifyType::Minify,
             },
         }
     }
@@ -113,7 +128,7 @@ impl BuildChunkingContext {
         self.runtime_type
     }
 
-    pub fn minify_type(&self) -> Vc<MinifyType> {
+    pub fn minify_type(&self) -> MinifyType {
         self.minify_type
     }
 }

diff --git a/crates/turbopack-build/src/ecmascript/minify.rs b/crates/turbopack-build/src/ecmascript/minify.rs
@@ -1,6 +1,6 @@
 use std::{io::Write, sync::Arc};
 
-use anyhow::{Context, Result};
+use anyhow::{bail, Context, Result};
 use swc_core::{
     base::{try_with_handler, Compiler},
     common::{
@@ -62,9 +62,16 @@ async fn perform_minify(path: Vc<FileSystemPath>, code_vc: Vc<Code>) -> Result<V
         None,
     );
     let mut parser = Parser::new_from(lexer);
+    // TODO should use our own handler that emits issues instead.
     let program = try_with_handler(cm.clone(), Default::default(), |handler| {
         GLOBALS.set(&Default::default(), || {
-            let program = parser.parse_program().unwrap();
+            let program = match parser.parse_program() {
+                Ok(program) => program,
+                Err(_error) => {
+                    // TODO should emit an issue
+                    bail!("failed to parse source code")
+                }
+            };
             let comments = SingleThreadedComments::default();
             let unresolved_mark = Mark::new();
             let top_level_mark = Mark::new();

diff --git a/crates/turbopack-build/src/ecmascript/node/content.rs b/crates/turbopack-build/src/ecmascript/node/content.rs
@@ -88,7 +88,7 @@ impl EcmascriptBuildNodeChunkContent {
 
         let code = code.build().cell();
         if matches!(
-            &*this.chunking_context.await?.minify_type().await?,
+            this.chunking_context.await?.minify_type(),
             MinifyType::Minify
         ) {
             return Ok(minify(chunk_path_vc, code));

diff --git a/crates/turbopack-cli/src/build/mod.rs b/crates/turbopack-cli/src/build/mod.rs
@@ -119,7 +119,7 @@ impl TurbopackBuildBuilder {
                 )
                 .cell(),
                 self.browserslist_query,
-                self.minify_type.cell(),
+                self.minify_type,
             );
 
             // Await the result to propagate any errors.
@@ -158,7 +158,7 @@ async fn build_internal(
     root_dir: String,
     entry_requests: Vc<EntryRequests>,
     browserslist_query: String,
-    minify_type: Vc<MinifyType>,
+    minify_type: MinifyType,
 ) -> Result<Vc<()>> {
     let env = Environment::new(Value::new(ExecutionEnvironment::Browser(
         BrowserEnvironment {

diff --git a/crates/turbopack-tests/tests/snapshot.rs b/crates/turbopack-tests/tests/snapshot.rs
@@ -299,7 +299,7 @@ async fn run_test(resource: String) -> Result<Vc<FileSystemPath>> {
                 static_root_path,
                 env,
             )
-            .minify_type(options.minify_type.into())
+            .minify_type(options.minify_type)
             .runtime_type(options.runtime_type)
             .build(),
         ),