Repository: oxc-project/oxc-resolver (rust)

Issue:
Unexpected result `NotFound('node:http')` with `imports` field and `builtins: true` option
```js
import fs from '#fs'
import http from '#http'
fs.readFileSync()
http.createServer()
```

```json
{
  "imports": {
    "#fs": {
      "node": "fs",
      "default": "./empty.js"
    },
    "#http": {
      "node": "node:http",
      "default": "./empty.js"
    }
  }
}
```

While resolving `#fs`, oxc-resolver correctly maps `#fs` to `fs`. Then oxc-resolver returns `NotFound("fs")`, which is also correct with `builtins: false`.

However, with `builtin: true`, oxc-resolver should return `Builtin('fs')` or `Builtins('node:http')`.

Pull request description:
feat!: return `ResolveError:Builtin("node:{specifier}")` from package imports and exports
closes #164

According to the ESM specification https://nodejs.org/api/esm.html#resolution-and-loading-algorithm

```
// PACKAGE_RESOLVE(packageSpecifier, parentURL)
// 3. If packageSpecifier is a Node.js builtin module name, then
//   1. Return the string "node:" concatenated with packageSpecifier.
```

The returned value must be prefixed by `node:`

Patch:
diff --git a/src/lib.rs b/src/lib.rs
index 2d603cf..1d54225 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -308,11 +308,19 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
         }
     }
 
+    // PACKAGE_RESOLVE(packageSpecifier, parentURL)
+    // 3. If packageSpecifier is a Node.js builtin module name, then
+    //   1. Return the string "node:" concatenated with packageSpecifier.
     fn require_core(&self, specifier: &str) -> Result<(), ResolveError> {
-        if self.options.builtin_modules
-            && (specifier.starts_with("node:") || NODEJS_BUILTINS.binary_search(&specifier).is_ok())
-        {
-            return Err(ResolveError::Builtin(specifier.to_string()));
+        if self.options.builtin_modules {
+            let starts_with_node = specifier.starts_with("node:");
+            if starts_with_node || NODEJS_BUILTINS.binary_search(&specifier).is_ok() {
+                let mut specifier = specifier.to_string();
+                if !starts_with_node {
+                    specifier = format!("node:{specifier}");
+                }
+                return Err(ResolveError::Builtin(specifier));
+            }
         }
         Ok(())
     }
@@ -1118,6 +1126,11 @@ impl<Fs: FileSystem> ResolverGeneric<Fs> {
         ctx: &mut Ctx,
     ) -> ResolveResult {
         let (package_name, subpath) = Self::parse_package_specifier(specifier);
+
+        // 3. If packageSpecifier is a Node.js builtin module name, then
+        //   1. Return the string "node:" concatenated with packageSpecifier.
+        self.require_core(package_name)?;
+
         // 11. While parentURL is not the file system root,
         for module_name in &self.options.modules {
             for cached_path in std::iter::successors(Some(cached_path), |p| p.parent()) {


Test patch:
diff --git a/fixtures/enhanced_resolve/test/fixtures/builtins/package.json b/fixtures/enhanced_resolve/test/fixtures/builtins/package.json
new file mode 100644
index 0000000..4a54ea9
--- /dev/null
+++ b/fixtures/enhanced_resolve/test/fixtures/builtins/package.json
@@ -0,0 +1,12 @@
+{
+  "name": "builtins",
+  "private": true,
+  "imports": {
+    "#fs": {
+      "default": "fs"
+    },
+    "#http": {
+      "node": "node:http"
+    }
+  }
+}
diff --git a/src/tests/builtins.rs b/src/tests/builtins.rs
index 6163dd1..5d6d14d 100644
--- a/src/tests/builtins.rs
+++ b/src/tests/builtins.rs
@@ -14,8 +14,7 @@ fn builtins_off() {
 fn builtins() {
     let f = Path::new("/");
 
-    let resolver =
-        Resolver::new(ResolveOptions { builtin_modules: true, ..ResolveOptions::default() });
+    let resolver = Resolver::new(ResolveOptions::default().with_builtin_modules(true));
 
     let pass = [
         "_http_agent",
@@ -86,13 +85,37 @@ fn builtins() {
     ];
 
     for request in pass {
-        let resolved_path = resolver.resolve(f, request).map(|r| r.full_path());
-        assert_eq!(resolved_path, Err(ResolveError::Builtin(request.to_string())), "{request}");
+        let prefixed_request = format!("node:{request}");
+        for request in [prefixed_request.clone(), request.to_string()] {
+            let resolved_path = resolver.resolve(f, &request).map(|r| r.full_path());
+            let err = ResolveError::Builtin(prefixed_request.clone());
+            assert_eq!(resolved_path, Err(err), "{request}");
+        }
     }
+}
 
-    for request in pass {
-        let request = format!("node:{request}");
-        let resolved_path = resolver.resolve(f, &request).map(|r| r.full_path());
-        assert_eq!(resolved_path, Err(ResolveError::Builtin(request.to_string())), "{request}");
+#[test]
+fn fail() {
+    let f = Path::new("/");
+    let resolver = Resolver::new(ResolveOptions::default().with_builtin_modules(true));
+    let request = "xxx";
+    let resolved_path = resolver.resolve(f, request);
+    let err = ResolveError::NotFound(request.to_string());
+    assert_eq!(resolved_path, Err(err), "{request}");
+}
+
+#[test]
+fn imports() {
+    let f = super::fixture().join("builtins");
+    let resolver = Resolver::new(ResolveOptions {
+        builtin_modules: true,
+        condition_names: vec!["node".into()],
+        ..ResolveOptions::default()
+    });
+
+    for request in ["#fs", "#http"] {
+        let resolved_path = resolver.resolve(f.clone(), request).map(|r| r.full_path());
+        let err = ResolveError::Builtin(format!("node:{}", request.trim_start_matches('#')));
+        assert_eq!(resolved_path, Err(err));
     }
 }