Fix postcss nitpicks (#3077)

Re: #3065

It also fixes the gnarly stack trace error whenever the node code throws, but it doesn't look very pretty. Not sure how to handle it:

<img width="1280" alt="Screen Shot 2022-12-19 at 3 46 56 PM" src="https://user-images.githubusercontent.com/112982/208519095-ec7ad539-5c23-4b95-bcc1-71ba30381e60.png">

crates/next-core/src/next_config.rs

@@ -7,12 +7,15 @@ use turbo_tasks::{
     trace::TraceRawVcs,
     Value,
 };
-use turbo_tasks_fs::FileSystemEntryType;
 use turbopack::evaluate_context::node_evaluate_asset_context;
 use turbopack_core::{
     asset::Asset,
     reference_type::{EntryReferenceSubType, ReferenceType},
-    resolve::options::{ImportMap, ImportMapping},
+    resolve::{
+        find_context_file,
+        options::{ImportMap, ImportMapping},
+        FindContextFileResult,
+    },
     source_asset::SourceAssetVc,
 };
 use turbopack_ecmascript::{
@@ -27,7 +30,7 @@ use turbopack_node::{
 use crate::embed_js::next_asset;
 
 #[turbo_tasks::value(serialization = "custom")]
-#[derive(Debug, Serialize, Deserialize, Clone)]
+#[derive(Debug, Serialize, Deserialize, Clone, Default)]
 #[serde(rename_all = "camelCase")]
 pub struct NextConfig {
     pub config_file: Option<String>,
@@ -200,6 +203,15 @@ impl NextConfigVc {
     }
 }
 
+fn next_configs() -> StringsVc {
+    StringsVc::cell(
+        ["next.config.mjs", "next.config.js"]
+            .into_iter()
+            .map(ToOwned::to_owned)
+            .collect(),
+    )
+}
+
 #[turbo_tasks::function]
 pub async fn load_next_config(execution_context: ExecutionContextVc) -> Result<NextConfigVc> {
     let ExecutionContext {
@@ -212,20 +224,10 @@ pub async fn load_next_config(execution_context: ExecutionContextVc) -> Result<N
     import_map.insert_wildcard_alias("next/", ImportMapping::External(None).into());
 
     let context = node_evaluate_asset_context(Some(import_map.cell()));
-    let next_config_mjs_path = project_root.join("next.config.mjs").realpath();
-    let next_config_js_path = project_root.join("next.config.js").realpath();
-    let config_asset = if matches!(
-        &*next_config_mjs_path.get_type().await?,
-        FileSystemEntryType::File
-    ) {
-        Some(SourceAssetVc::new(next_config_mjs_path))
-    } else if matches!(
-        &*next_config_js_path.get_type().await?,
-        FileSystemEntryType::File
-    ) {
-        Some(SourceAssetVc::new(next_config_js_path))
-    } else {
-        None
+    let find_config_result = find_context_file(project_root, next_configs());
+    let config_asset = match &*find_config_result.await? {
+        FindContextFileResult::Found(config_path, _) => Some(SourceAssetVc::new(*config_path)),
+        FindContextFileResult::NotFound(_) => None,
     };
 
     let runtime_entries = config_asset.map(|config_asset| {
@@ -262,6 +264,7 @@ pub async fn load_next_config(execution_context: ExecutionContextVc) -> Result<N
             let next_config: NextConfig = serde_json::from_reader(val.read())?;
             Ok(next_config.cell())
         }
+        JavaScriptValue::Error => Ok(NextConfig::default().cell()),
         JavaScriptValue::Stream(_) => {
             unimplemented!("Stream not supported now");
         }

crates/turbo-tasks/src/completion.rs

@@ -24,9 +24,9 @@ impl CompletionVc {
     /// Uses the previous completion. Can be used to cancel without triggering a
     /// new invalidation.
     pub fn unchanged() -> Self {
-        // This is the same code that CompletionVc::cell uses expect that it
-        // infact compares the cell (CompletionVc::cell opted-out of
-        // that via #[turbo_tasks::value(cell = "new")])
+        // This is the same code that CompletionVc::cell uses except that it
+        // in fact compares the cell (CompletionVc::cell opted-out of
+        // that via `#[turbo_tasks::value(cell = "new")]`)
         let cell = turbo_tasks::macro_helpers::find_cell_by_type(*COMPLETIONS_VALUE_TYPE_ID);
         cell.compare_and_update_shared(Completion);
         let raw: RawVc = cell.into();

crates/turbopack-core/src/source_asset.rs

@@ -1,7 +1,10 @@
 use anyhow::Result;
 use turbo_tasks_fs::{FileContent, FileSystemEntryType, FileSystemPathVc, LinkContent};
 
-use crate::asset::{Asset, AssetContent, AssetContentVc, AssetVc};
+use crate::{
+    asset::{Asset, AssetContent, AssetContentVc, AssetVc},
+    reference::AssetReferencesVc,
+};
 
 /// The raw [Asset]. It represents raw content from a path without any
 /// references to other [Asset]s.
@@ -44,4 +47,11 @@ impl Asset for SourceAsset {
             _ => Err(anyhow::anyhow!("Invalid file type {:?}", file_type)),
         }
     }
+
+    #[turbo_tasks::function]
+    fn references(&self) -> AssetReferencesVc {
+        // TODO: build input sourcemaps via language specific sourceMappingURL comment
+        // or parse.
+        AssetReferencesVc::empty()
+    }
 }

crates/turbopack-ecmascript/js/src/runtime.js

@@ -191,8 +191,7 @@
       // an external module we don't provide a shim for (e.g. querystring, url).
       // For now, we fail semi-silently, but in the future this should be a
       // compilation error.
-      console.error(`Failed to load external module ${id}: ${err}`);
-      return undefined;
+      throw new Error(`Failed to load external module ${id}: ${err}`);
     }
     if (!esm || raw.__esModule) {
       return raw;

crates/turbopack-ecmascript/src/references/esm/base.rs

@@ -216,7 +216,7 @@ impl CodeGenerateable for EsmAssetReference {
                         visitors.push(create_visitor!(visit_mut_program(program: &mut Program) {
                             // TODO Technically this should insert a ESM external, but we don't support that yet
                             let stmt = quote!(
-                                "var $name = __turbopack_external_require__($id, 1);" as Stmt,
+                                "var $name = __turbopack_external_require__($id, true);" as Stmt,
                                 name = Ident::new(ident.clone().into(), DUMMY_SP),
                                 id: Expr = Expr::Lit(request.clone().into())
                             );

crates/turbopack-node/js/src/ipc/evaluate.ts

@@ -18,16 +18,15 @@ export const run = async (getValue: (...deserializedArgs: any[]) => any) => {
 
     switch (msg.type) {
       case "evaluate": {
-        const value = await Promise.resolve()
-          .then(() => getValue(...msg.args))
-          .catch((err: Error) => {
-            // sendError will exit the process
-            return ipc.sendError(err);
+        try {
+          const value = await getValue(...msg.args);
+          await ipc.send({
+            type: "jsonValue",
+            data: JSON.stringify(value),
           });
-        await ipc.send({
-          type: "jsonValue",
-          data: JSON.stringify(value),
-        });
+        } catch (e) {
+          await ipc.sendError(e as Error);
+        }
         break;
       }
       default: {

crates/turbopack-node/src/evaluate.rs

@@ -1,12 +1,16 @@
 use std::{collections::HashMap, thread::available_parallelism};
 
-use anyhow::{bail, Result};
-use turbo_tasks::{primitives::JsonValueVc, TryJoinIterExt, Value};
+use anyhow::Result;
+use turbo_tasks::{
+    primitives::{JsonValueVc, StringVc},
+    TryJoinIterExt, Value,
+};
 use turbo_tasks_fs::{rope::Rope, to_sys_path, File, FileSystemPathVc};
 use turbopack_core::{
     asset::AssetVc,
     chunk::{dev::DevChunkingContextVc, ChunkGroupVc},
     context::AssetContextVc,
+    issue::{Issue, IssueVc},
     source_asset::SourceAssetVc,
     virtual_asset::VirtualAssetVc,
 };
@@ -20,12 +24,13 @@ use crate::{
     embed_js::embed_file_path,
     emit,
     pool::{NodeJsOperation, NodeJsPool, NodeJsPoolVc},
-    EvalJavaScriptIncomingMessage, EvalJavaScriptOutgoingMessage,
+    EvalJavaScriptIncomingMessage, EvalJavaScriptOutgoingMessage, StructuredError,
 };
 
 #[turbo_tasks::value(shared)]
 #[derive(Clone)]
 pub enum JavaScriptValue {
+    Error,
     Value(Rope),
     // TODO, support stream in the future
     Stream(#[turbo_tasks(trace_ignore)] Vec<u8>),
@@ -34,14 +39,22 @@ pub enum JavaScriptValue {
 async fn eval_js_operation(
     operation: &mut NodeJsOperation,
     content: EvalJavaScriptOutgoingMessage<'_>,
-) -> Result<String> {
+    context_path: FileSystemPathVc,
+) -> Result<JavaScriptValue> {
     operation.send(content).await?;
-    match operation.recv().await? {
-        EvalJavaScriptIncomingMessage::Error(err) => {
-            bail!(err.print(Default::default(), None).await?);
+    Ok(match operation.recv().await? {
+        EvalJavaScriptIncomingMessage::Error(error) => {
+            EvaluationIssue {
+                error,
+                context_path,
+            }
+            .cell()
+            .as_issue()
+            .emit();
+            JavaScriptValue::Error
         }
-        EvalJavaScriptIncomingMessage::JsonValue { data } => Ok(data),
-    }
+        EvalJavaScriptIncomingMessage::JsonValue { data } => JavaScriptValue::Value(data.into()),
+    })
 }
 
 #[turbo_tasks::function]
@@ -113,9 +126,9 @@ pub async fn get_evaluate_pool(
     Ok(pool.cell())
 }
 
-#[turbo_tasks::function]
 /// Pass the file you cared as `runtime_entries` to invalidate and reload the
 /// evaluated result automatically.
+#[turbo_tasks::function]
 pub async fn evaluate(
     context_path: FileSystemPathVc,
     module_asset: AssetVc,
@@ -135,18 +148,51 @@ pub async fn evaluate(
     )
     .await?;
     let mut operation = pool.operation().await?;
-    let args = args.into_iter().map(|v| v).try_join().await?;
+    let args = args.into_iter().try_join().await?;
     let output = eval_js_operation(
         &mut operation,
         EvalJavaScriptOutgoingMessage::Evaluate {
             args: args.iter().map(|v| &**v).collect(),
         },
+        context_path,
     )
     .await?;
     if args.is_empty() {
         // Assume this is a one-off operation, so we can kill the process
         // TODO use a better way to decide that.
         operation.wait_or_kill().await?;
     }
-    Ok(JavaScriptValue::Value(output.into()).cell())
+    Ok(output.cell())
+}
+
+/// An issue that occurred while evaluating node code.
+#[turbo_tasks::value(shared)]
+pub struct EvaluationIssue {
+    pub context_path: FileSystemPathVc,
+    pub error: StructuredError,
+}
+
+#[turbo_tasks::value_impl]
+impl Issue for EvaluationIssue {
+    #[turbo_tasks::function]
+    fn title(&self) -> StringVc {
+        StringVc::cell("Error evaluating Node.js code".to_string())
+    }
+
+    #[turbo_tasks::function]
+    fn category(&self) -> StringVc {
+        StringVc::cell("build".to_string())
+    }
+
+    #[turbo_tasks::function]
+    fn context(&self) -> FileSystemPathVc {
+        self.context_path
+    }
+
+    #[turbo_tasks::function]
+    async fn description(&self) -> Result<StringVc> {
+        Ok(StringVc::cell(
+            self.error.print(Default::default(), None).await?,
+        ))
+    }
 }

crates/turbopack-node/src/transforms/postcss.rs

@@ -6,7 +6,7 @@ use turbo_tasks::{
     primitives::{JsonValueVc, StringsVc},
     TryJoinIterExt, Value,
 };
-use turbo_tasks_fs::{rope::Rope, File, FileContent, FileSystemEntryType, FileSystemPathVc};
+use turbo_tasks_fs::{File, FileContent, FileSystemEntryType, FileSystemPathVc};
 use turbopack_core::{
     asset::{Asset, AssetContent, AssetContentVc, AssetVc},
     context::AssetContextVc,
@@ -120,8 +120,8 @@ impl Asset for PostCssTransformedAsset {
             project_root,
             intermediate_output_path,
         } = *self.execution_context.await?;
-        let content = self.source.content().await?;
-        let AssetContent::File(file) = *content else {
+        let source_content = self.source.content();
+        let AssetContent::File(file) = *source_content.await? else {
             bail!("PostCSS transform only support transforming files");
         };
         let FileContent::Content(content) = &*file.await? else {
@@ -186,12 +186,11 @@ impl Asset for PostCssTransformedAsset {
         )
         .await?;
         let JavaScriptValue::Value(val) = &*config_value else {
-            bail!("Expected a value from PostCSS transform");
+            return Ok(source_content);
         };
         let processed_css: ProcessedCSS = serde_json::from_reader(val.read())
             .context("Unable to deserializate response from PostCSS transform operation")?;
-        let new_content = Rope::from(processed_css.css.clone());
-        let file = File::from(new_content);
+        let file = File::from(processed_css.css);
         // TODO handle SourceMap
         Ok(AssetContent::File(FileContent::Content(file).cell()).cell())
     }