From 91fd375a3b811b323baf3f0cc847ddaf142d38c8 Mon Sep 17 00:00:00 2001
From: Boshen <boshenc@gmail.com>
Date: Wed, 9 Aug 2023 14:28:44 +0800
Subject: [PATCH] refactor(resolver): return package json error immediately
 instead of saving it (#702)

The error is propagated so there is no need to save it.
---
 Cargo.lock                       |  1 +
 crates/oxc_resolver/Cargo.toml   |  2 ++
 crates/oxc_resolver/src/cache.rs | 29 +++++++++++++++--------------
 crates/oxc_resolver/src/lib.rs   |  8 +++-----
 4 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 36c9296db..8dadfefe4 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1756,6 +1756,7 @@ dependencies = [
  "jemallocator",
  "mimalloc",
  "nodejs-resolver",
+ "once_cell",
  "rayon",
  "rustc-hash",
  "serde",
diff --git a/crates/oxc_resolver/Cargo.toml b/crates/oxc_resolver/Cargo.toml
index f67d314cc..2ddf27b29 100644
--- a/crates/oxc_resolver/Cargo.toml
+++ b/crates/oxc_resolver/Cargo.toml
@@ -20,6 +20,8 @@ rustc-hash    = { workspace = true }
 indexmap      = { workspace = true, features = ["serde"] }
 dunce         = "1.0.4"
 identity-hash = "0.1.0"
+# Use `std::sync::OnceLock::get_or_try_init` when it is stable.
+once_cell     = "1.18.0"
 
 [dev-dependencies]
 static_assertions = { workspace = true }
diff --git a/crates/oxc_resolver/src/cache.rs b/crates/oxc_resolver/src/cache.rs
index 76e084fb4..7a910789f 100644
--- a/crates/oxc_resolver/src/cache.rs
+++ b/crates/oxc_resolver/src/cache.rs
@@ -1,9 +1,10 @@
+use once_cell::sync::OnceCell as OnceLock;
 use std::{
     convert::AsRef,
     hash::{Hash, Hasher},
     ops::Deref,
     path::{Path, PathBuf},
-    sync::{Arc, OnceLock},
+    sync::Arc,
 };
 
 use dashmap::DashMap;
@@ -68,7 +69,7 @@ pub struct CachedPathImpl {
     parent: Option<CachedPath>,
     meta: OnceLock<Option<FileMetadata>>,
     symlink: OnceLock<Option<PathBuf>>,
-    package_json: OnceLock<Option<Result<Arc<PackageJson>, ResolveError>>>,
+    package_json: OnceLock<Option<Arc<PackageJson>>>,
 }
 
 impl CachedPathImpl {
@@ -124,7 +125,7 @@ impl CachedPathImpl {
         }
         let mut cache_value = Some(cache_value);
         while let Some(cv) = cache_value {
-            if let Some(package_json) = cv.package_json(fs).transpose()? {
+            if let Some(package_json) = cv.package_json(fs)? {
                 return Ok(Some(Arc::clone(&package_json)));
             }
             cache_value = cv.parent.as_deref();
@@ -140,19 +141,19 @@ impl CachedPathImpl {
     pub fn package_json<Fs: FileSystem>(
         &self,
         fs: &Fs,
-    ) -> Option<Result<Arc<PackageJson>, ResolveError>> {
-        // Change to `get_or_try_init` once it is stable
+    ) -> Result<Option<Arc<PackageJson>>, ResolveError> {
+        // Change to `std::sync::OnceLock::get_or_try_init` when it is stable.
         self.package_json
-            .get_or_init(|| {
+            .get_or_try_init(|| {
                 let package_json_path = self.path.join("package.json");
-                fs.read_to_string(&package_json_path).ok().map(|package_json_string| {
-                    PackageJson::parse(package_json_path.clone(), &package_json_string)
-                        .map(Arc::new)
-                        .map_err(|error| {
-                            ResolveError::from_serde_json_error(package_json_path, &error)
-                        })
-                })
+                let Ok(package_json_string) = fs.read_to_string(&package_json_path) else {
+                    return Ok(None)
+                };
+                PackageJson::parse(package_json_path.clone(), &package_json_string)
+                    .map(Arc::new)
+                    .map(Some)
+                    .map_err(|error| ResolveError::from_serde_json_error(package_json_path, &error))
             })
-            .clone()
+            .cloned()
     }
 }
diff --git a/crates/oxc_resolver/src/lib.rs b/crates/oxc_resolver/src/lib.rs
index 4ed32f572..0a04f36a9 100644
--- a/crates/oxc_resolver/src/lib.rs
+++ b/crates/oxc_resolver/src/lib.rs
@@ -435,7 +435,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
         // 1. If X/package.json is a file,
         if !self.options.description_files.is_empty() {
             // a. Parse X/package.json, and look for "main" field.
-            if let Some(package_json) = cached_path.package_json(&self.cache.fs).transpose()? {
+            if let Some(package_json) = cached_path.package_json(&self.cache.fs)? {
                 // b. If "main" is a falsy value, GOTO 2.
                 if let Some(main_field) = &package_json.main {
                     // c. let M = X + (json main field)
@@ -512,7 +512,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
         //    return.
         let (name, subpath) = Self::parse_package_specifier(specifier);
         let cached_path = self.cache.value(&path.join(name));
-        let Some(package_json) = cached_path.package_json(&self.cache.fs).transpose()? else {
+        let Some(package_json) = cached_path.package_json(&self.cache.fs)? else {
             return Ok(None);
         };
         // 3. Parse DIR/NAME/package.json, and look for "exports" field.
@@ -705,9 +705,7 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
                 //   1. Continue the next loop iteration.
                 if cached_path.is_dir(&self.cache.fs) {
                     // 4. Let pjson be the result of READ_PACKAGE_JSON(packageURL).
-                    if let Some(package_json) =
-                        cached_path.package_json(&self.cache.fs).transpose()?
-                    {
+                    if let Some(package_json) = cached_path.package_json(&self.cache.fs)? {
                         // 5. If pjson is not null and pjson.exports is not null or undefined, then
                         if !package_json.exports.is_none() {
                             // 1. Return the result of PACKAGE_EXPORTS_RESOLVE(packageURL, packageSubpath, pjson.exports, defaultConditions).