feat(resolver): add thiserror (#847)

Cargo.lock

@@ -1774,6 +1774,7 @@ dependencies = [
  "rustc-hash",
  "serde",
  "serde_json",
+ "thiserror",
  "tracing",
  "tracing-subscriber",
  "vfs",

crates/oxc_resolver/Cargo.toml

@@ -27,6 +27,7 @@ indexmap = { workspace = true, features = ["serde"] } # serde for Deserialize fr
 dunce = "1.0.4"
 # Use `std::sync::OnceLock::get_or_try_init` when it is stable.
 once_cell = "1.18.0"
+thiserror = { workspace = true }
 
 [dev-dependencies]
 vfs                = "0.9.0"              # for testing with in memory file system

crates/oxc_resolver/README.md

@@ -3,10 +3,6 @@
 * [enhanced-resolve](https://github.com/webpack/enhanced-resolve) configurations
 * built-in [tsconfig-paths-webpack-plugin](https://github.com/dividab/tsconfig-paths-webpack-plugin)
 
-## TODO
-
-- [ ] use `thiserror` for better error messages
-
 #### Resolver Options
 
 | Done | Field            | Default                     | Description                                                                                                                                               |

crates/oxc_resolver/examples/resolver.rs

@@ -37,7 +37,9 @@ fn main() {
         alias: vec![("/asdf".into(), vec![AliasValue::Path("./test.js".into())])],
         ..ResolveOptions::default()
     };
-    let resolved_path = Resolver::new(options).resolve(path, &request);
 
-    println!("Result: {resolved_path:?}");
+    match Resolver::new(options).resolve(path, &request) {
+        Err(error) => println!("Error: {error}"),
+        Ok(resolution) => println!("Resolved: {}", resolution.full_path().to_string_lossy()),
+    }
 }

crates/oxc_resolver/src/error.rs

@@ -1,7 +1,8 @@
 use std::path::PathBuf;
+use thiserror::Error;
 
 /// All resolution errors.
-#[derive(Debug, Clone, Eq, PartialEq)]
+#[derive(Debug, Clone, Eq, PartialEq, Error)]
 pub enum ResolveError {
     /// Ignored path
     ///
@@ -14,53 +15,70 @@ pub enum ResolveError {
     /// }
     /// ```
     /// See <https://github.com/defunctzombie/package-browser-field-spec#ignore-a-module>
+    #[error("Path is ignored")]
     Ignored(PathBuf),
 
     /// Path not found
+    #[error("Path not found {0}")]
     NotFound(PathBuf),
 
     /// Node.js builtin modules
     ///
     /// This is an error due to not being a Node.js runtime.
     /// The `alias` option can be used to resolve a builtin module to a polyfill.
+    #[error("Builtin module")]
     Builtin(String),
 
     /// All of the aliased extension are not found
+    #[error("All of the aliased extension are not found")]
     ExtensionAlias,
 
     /// The provided path specifier cannot be parsed
+    #[error("{0}")]
     Specifier(SpecifierError),
 
     /// JSON parse error
+    #[error("{0:?}")]
     JSON(JSONError),
 
     /// Restricted by `ResolveOptions::restrictions`
+    #[error("Restriction")]
     Restriction(PathBuf),
 
     // TODO: TypeError [ERR_INVALID_MODULE_SPECIFIER]: Invalid module "./dist/../../../a.js" specifier is not a valid subpath for the "exports" resolution of /xxx/package.json
+    #[error("[ERR_INVALID_MODULE_SPECIFIER]: Invalid module specifier \"{0}\"")]
     InvalidModuleSpecifier(String),
 
     // TODO: Error [ERR_INVALID_PACKAGE_TARGET]: Invalid "exports" target "./../../a.js" defined for './dist/a.js' in the package config /xxx/package.json
+    #[error("[ERR_INVALID_PACKAGE_TARGET]: Invalid \"exports\" target \"{0}\"")]
     InvalidPackageTarget(String),
 
     // TODO: Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './anything/else' is not defined by "exports" in /xxx/package.json
+    #[error(
+        "[ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath '{0}' is not defined by \"exports\""
+    )]
     PackagePathNotExported(String),
 
     // TODO: Invalid package config /xxx/package.json. "exports" cannot contain some keys starting with '.' and some not. The exports object must either be an object of package subpath keys or an object of main entry condition name keys only.
+    #[error("Invalid package config")]
     InvalidPackageConfig(PathBuf),
 
     // TODO: Default condition should be last one
+    #[error("Default condition should be last one")]
     InvalidPackageConfigDefault(PathBuf),
 
-    // TODO:  Expecting folder to folder mapping. "./data/timezones" should end with "/"
+    // TODO: Expecting folder to folder mapping. "./data/timezones" should end with "/"
+    #[error("Expecting folder to folder mapping. \"{0}\" should end with \"/\"")]
     InvalidPackageConfigDirectory(PathBuf),
 
+    #[error("Package import not defined")]
     PackageImportNotDefined(String),
 
+    #[error("{0} is unimplemented")]
     Unimplemented(&'static str),
 
-    // enhanced-resolve Error: Recursion in resolving
-    // Occurs when alias paths reference each other.
+    /// Occurs when alias paths reference each other.
+    #[error("Recursion in resolving")]
     Recursion,
 }
 
@@ -70,8 +88,9 @@ impl ResolveError {
     }
 }
 
-#[derive(Debug, Clone, Eq, PartialEq)]
+#[derive(Debug, Clone, Eq, PartialEq, Error)]
 pub enum SpecifierError {
+    #[error("[ERR_INVALID_ARG_VALUE]: The specifiers must be a non-empty string. Received ''")]
     Empty,
 }
 

crates/oxc_resolver/src/lib.rs

@@ -527,9 +527,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
                     }
                 }
                 Restriction::RegExp(_) => {
-                    return Err(ResolveError::Unimplemented(
-                        "Restriction with regex is unimplemented.",
-                    ))
+                    return Err(ResolveError::Unimplemented("Restriction with regex"))
                 }
             }
         }