feat: collect errors from NormalModuleTask (#298)

<!-- Thank you for contributing! -->

### Description

<!-- Please insert your description here and provide especially info about the "what" this PR is solving -->

### Test Plan

<!-- e.g. is there anything you'd like reviewers to focus on? -->

---

crates/rolldown/src/bundler/module_loader/mod.rs

@@ -7,10 +7,13 @@ mod task_result;
 
 pub use module_loader::ModuleLoader;
 
+use crate::error::BatchedErrors;
+
 use self::{
   runtime_normal_module_task::RuntimeNormalModuleTaskResult, task_result::NormalModuleTaskResult,
 };
 pub enum Msg {
   NormalModuleDone(NormalModuleTaskResult),
   RuntimeNormalModuleDone(RuntimeNormalModuleTaskResult),
+  Errors(BatchedErrors),
 }

crates/rolldown/src/bundler/module_loader/module_loader.rs

@@ -18,7 +18,7 @@ use crate::bundler::plugin_driver::SharedPluginDriver;
 use crate::bundler::runtime::RuntimeModuleBrief;
 use crate::bundler::utils::resolve_id::ResolvedRequestInfo;
 use crate::bundler::utils::symbols::Symbols;
-use crate::error::BatchedResult;
+use crate::error::{BatchedErrors, BatchedResult};
 use crate::SharedResolver;
 
 pub struct ModuleLoader<T: FileSystem + Default> {
@@ -124,6 +124,8 @@ impl<T: FileSystem + 'static + Default> ModuleLoader<T> {
   ) -> BatchedResult<ModuleLoaderOutput> {
     assert!(!self.input_options.input.is_empty(), "You must supply options.input to rolldown");
 
+    let mut errors = BatchedErrors::default();
+
     self.intermediate_modules.reserve(user_defined_entries.len() + 1 /* runtime */);
 
     let mut entries: Vec<(Option<String>, ModuleId)> = user_defined_entries
@@ -163,8 +165,6 @@ impl<T: FileSystem + 'static + Default> ModuleLoader<T> {
             })
             .collect::<IndexVec<ImportRecordId, _>>();
           builder.import_records = Some(import_records);
-          builder.pretty_path =
-            Some(builder.path.as_ref().unwrap().prettify(&self.input_options.cwd));
           self.intermediate_modules[module_id] = Some(Module::Normal(builder.build()));
 
           self.symbols.add_ast_symbol(module_id, ast_symbol);
@@ -177,10 +177,17 @@ impl<T: FileSystem + 'static + Default> ModuleLoader<T> {
           self.symbols.add_ast_symbol(runtime.id(), ast_symbol);
           runtime_brief = Some(runtime);
         }
+        Msg::Errors(errs) => {
+          errors.merge(errs);
+        }
       }
       self.remaining -= 1;
     }
 
+    if !errors.is_empty() {
+      return Err(errors);
+    }
+
     let modules = self.intermediate_modules.into_iter().map(Option::unwrap).collect();
 
     let mut dynamic_entries = Vec::from_iter(dynamic_entries);

crates/rolldown/src/bundler/module_loader/normal_module_task.rs

@@ -6,7 +6,6 @@ use oxc::{ast::Visit, span::SourceType};
 use rolldown_common::{
   FilePath, ImportRecordId, ModuleId, ModuleType, RawImportRecord, ResourceId, SymbolRef,
 };
-use rolldown_error::BuildError;
 use rolldown_fs::FileSystem;
 use rolldown_oxc::{OxcCompiler, OxcProgram};
 use rolldown_resolver::Resolver;
@@ -34,8 +33,6 @@ pub struct NormalModuleTask<'task, T: FileSystem + Default> {
   module_id: ModuleId,
   path: FilePath,
   module_type: ModuleType,
-  errors: Vec<BuildError>,
-  warnings: Vec<BuildError>,
   is_entry: bool,
 }
 
@@ -47,22 +44,19 @@ impl<'task, T: FileSystem + Default + 'static> NormalModuleTask<'task, T> {
     path: FilePath,
     module_type: ModuleType,
   ) -> Self {
-    Self {
-      ctx,
-      module_id: id,
-      is_entry,
-      path,
-      module_type,
-      errors: Vec::default(),
-      warnings: Vec::default(),
+    Self { ctx, module_id: id, is_entry, path, module_type }
+  }
+  pub async fn run(mut self) {
+    if let Err(errs) = self.run_inner().await {
+      self.ctx.tx.send(Msg::Errors(errs)).expect("Send should not fail");
     }
   }
 
-  #[tracing::instrument(skip_all)]
-  pub async fn run(mut self) -> BatchedResult<()> {
-    let mut builder = NormalModuleBuilder::default();
+  async fn run_inner(&mut self) -> BatchedResult<()> {
     tracing::trace!("process {:?}", self.path);
 
+    let warnings = vec![];
+
     // Run plugin load to get content first, if it is None using read fs as fallback.
     let mut source = if let Some(r) =
       self.ctx.plugin_driver.load(&HookLoadArgs { id: self.path.as_ref() }).await?
@@ -98,34 +92,38 @@ impl<'task, T: FileSystem + Default + 'static> NormalModuleTask<'task, T> {
       unique_name,
     } = scan_result;
 
-    builder.id = Some(self.module_id);
-    builder.ast = Some(ast);
-    builder.unique_name = Some(unique_name);
-    builder.path = Some(ResourceId::new(self.path));
-    builder.named_imports = Some(named_imports);
-    builder.named_exports = Some(named_exports);
-    builder.stmt_infos = Some(stmt_infos);
-    builder.imports = Some(imports);
-    builder.star_exports = Some(star_exports);
-    builder.default_export_symbol = export_default_symbol_id;
-    builder.scope = Some(scope);
-    builder.exports_kind = exports_kind;
-    builder.namespace_symbol = Some(namespace_symbol);
-    builder.module_type = self.module_type;
-    builder.is_entry = self.is_entry;
+    let builder = NormalModuleBuilder {
+      id: Some(self.module_id),
+      ast: Some(ast),
+      unique_name: Some(unique_name),
+      path: Some(ResourceId::new(self.path.clone())),
+      named_imports: Some(named_imports),
+      named_exports: Some(named_exports),
+      stmt_infos: Some(stmt_infos),
+      imports: Some(imports),
+      star_exports: Some(star_exports),
+      default_export_symbol: export_default_symbol_id,
+      scope: Some(scope),
+      exports_kind,
+      namespace_symbol: Some(namespace_symbol),
+      module_type: self.module_type,
+      is_entry: self.is_entry,
+      pretty_path: Some(self.path.prettify(&self.ctx.input_options.cwd)),
+      ..Default::default()
+    };
+
     self
       .ctx
       .tx
       .send(Msg::NormalModuleDone(NormalModuleTaskResult {
         resolved_deps: res,
         module_id: self.module_id,
-        errors: self.errors,
-        warnings: self.warnings,
+        warnings,
         ast_symbol,
         builder,
         raw_import_records: import_records,
       }))
-      .unwrap();
+      .expect("Send should not fail");
     Ok(())
   }
 

crates/rolldown/src/bundler/module_loader/task_result.rs

@@ -11,7 +11,6 @@ pub struct NormalModuleTaskResult {
   pub ast_symbol: AstSymbol,
   pub resolved_deps: IndexVec<ImportRecordId, ResolvedRequestInfo>,
   pub raw_import_records: IndexVec<ImportRecordId, RawImportRecord>,
-  pub errors: Vec<BuildError>,
   pub warnings: Vec<BuildError>,
   pub builder: NormalModuleBuilder,
 }

crates/rolldown_common/src/file_path.rs

@@ -63,6 +63,20 @@ impl FilePath {
     name
   }
 
+  pub fn prettify(&self, cwd: impl AsRef<Path>) -> String {
+    let pretty = if Path::new(self.0.as_ref()).is_absolute() {
+      Path::new(self.0.as_ref())
+        .relative(cwd.as_ref())
+        .into_os_string()
+        .into_string()
+        .expect("should be valid utf8")
+    } else {
+      self.0.to_string()
+    };
+    // remove \0
+    pretty.replace('\0', "")
+  }
+
   // This doesn't ensure uniqueness, but should be valid as a JS identifier.
   pub fn representative_name(&self) -> Cow<str> {
     let path = Path::new(self.0.as_ref());

crates/rolldown_common/src/module_path.rs

@@ -1,6 +1,4 @@
-use std::{fmt::Debug, path::Path};
-
-use sugar_path::SugarPath;
+use std::fmt::Debug;
 
 use crate::FilePath;
 
@@ -12,21 +10,8 @@ impl ResourceId {
     Self(path)
   }
 
+  // We may change `ResourceId` to enum in the future, so we have this method to make it easier to change.
   pub fn expect_file(&self) -> &FilePath {
     &self.0
   }
-
-  pub fn prettify(&self, cwd: impl AsRef<Path>) -> String {
-    let pretty = if Path::new(self.0.as_str()).is_absolute() {
-      Path::new(self.0.as_str())
-        .relative(cwd.as_ref())
-        .into_os_string()
-        .into_string()
-        .expect("should be valid utf8")
-    } else {
-      self.0.to_string()
-    };
-    // remove \0
-    pretty.replace('\0', "")
-  }
 }

package.json

@@ -12,11 +12,11 @@
   "scripts": {
     "lint": "oxlint --ignore-path=./.oxlintignore --import-plugin",
     "lint-filename": "ls-lint",
-    "build": "yarn workspaces foreach --all --parallel --topological-dev run build",
+    "build": "yarn workspaces foreach --all --topological-dev run build",
     "build:binding": "yarn workspace @rolldown/node-binding run build",
     "build:binding:release": "yarn workspace @rolldown/node-binding run build:release",
-    "test": "yarn workspaces foreach --all run test",
-    "test:update": "yarn workspaces foreach --all run test:update",
+    "test": "yarn workspaces foreach --all --parallel run test",
+    "test:update-rollup": "yarn workspace rollup-tests run test:update",
     "prettier": "prettier . '!**/*.{js||ts|json|md|yml|yaml}' -w",
     "prettier:ci": "prettier . '!**/*.{js||ts|json|md|yml|yaml}' -c",
     "bench": "yarn workspace bench run bench",