Skip to content

Commit 6684a64

Browse files
authored
refactor!: remove description_files option (oxc-project#488)
The only value for this option in the ecosystem is `package.json`. This option also slightly impacts performance.
1 parent 00bf216 commit 6684a64

6 files changed

Lines changed: 19 additions & 55 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,6 @@ See [index.d.ts](https://github.com/oxc-project/oxc-resolver/blob/main/napi/inde
170170
| aliasFields | [] | A list of alias fields in description files |
171171
| extensionAlias | {} | An object which maps extension to extension aliases |
172172
| conditionNames | [] | A list of exports field condition names |
173-
| descriptionFiles | ["package.json"] | A list of description files to read from |
174173
| enforceExtension | false | Enforce that a extension from extensions must be used |
175174
| exportsFields | ["exports"] | A list of exports fields in description files |
176175
| extensions | [".js", ".json", ".node"] | A list of extensions which should be tried for files |
@@ -190,6 +189,7 @@ See [index.d.ts](https://github.com/oxc-project/oxc-resolver/blob/main/napi/inde
190189

191190
| Field | Default | Description |
192191
| ---------------- | --------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------- |
192+
| descriptionFiles | ["package.json"] | A list of description files to read from |
193193
| modules | ["node_modules"] | A list of directories to resolve modules from, can be absolute path or folder name |
194194
| cachePredicate | function() { return true }; | A function which decides whether a request should be cached or not. An object is passed to the function with `path` and `request` properties. |
195195
| cacheWithContext | true | If unsafe cache is enabled, includes `request.context` in the cache key |

napi/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,6 @@ impl ResolverFactory {
156156
.map(|o| o.into_iter().map(|x| StrOrStrList(x).into()).collect::<Vec<_>>())
157157
.unwrap_or(default.alias_fields),
158158
condition_names: op.condition_names.unwrap_or(default.condition_names),
159-
description_files: op.description_files.unwrap_or(default.description_files),
160159
enforce_extension: op
161160
.enforce_extension
162161
.map(|enforce_extension| enforce_extension.into())

napi/src/options.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,6 @@ pub struct NapiResolveOptions {
3838
/// Default `[]`
3939
pub condition_names: Option<Vec<String>>,
4040

41-
/// The JSON files to use for descriptions. (There was once a `bower.json`.)
42-
///
43-
/// Default `["package.json"]`
44-
pub description_files: Option<Vec<String>>,
45-
4641
/// If true, it will not allow extension-less files.
4742
/// So by default `require('./foo')` works if `./foo` has a `.js` extension,
4843
/// but with this enabled only `require('./foo.js')` will work.

src/lib.rs

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -590,30 +590,26 @@ impl<C: Cache> ResolverGeneric<C> {
590590
}
591591

592592
fn load_as_directory(&self, cached_path: &C::Cp, ctx: &mut Ctx) -> ResolveResult<C::Cp> {
593-
// TODO: Only package.json is supported, so warn about having other values
594-
// Checking for empty files is needed for omitting checks on package.json
595593
// 1. If X/package.json is a file,
596-
if !self.options.description_files.is_empty() {
597-
// a. Parse X/package.json, and look for "main" field.
598-
if let Some((_, package_json)) =
599-
self.cache.get_package_json(cached_path, &self.options, ctx)?
600-
{
601-
// b. If "main" is a falsy value, GOTO 2.
602-
for main_field in package_json.main_fields(&self.options.main_fields) {
603-
// c. let M = X + (json main field)
604-
let cached_path = cached_path.normalize_with(main_field, self.cache.as_ref());
605-
// d. LOAD_AS_FILE(M)
606-
if let Some(path) = self.load_as_file(&cached_path, ctx)? {
607-
return Ok(Some(path));
608-
}
609-
// e. LOAD_INDEX(M)
610-
if let Some(path) = self.load_index(&cached_path, ctx)? {
611-
return Ok(Some(path));
612-
}
594+
// a. Parse X/package.json, and look for "main" field.
595+
if let Some((_, package_json)) =
596+
self.cache.get_package_json(cached_path, &self.options, ctx)?
597+
{
598+
// b. If "main" is a falsy value, GOTO 2.
599+
for main_field in package_json.main_fields(&self.options.main_fields) {
600+
// c. let M = X + (json main field)
601+
let cached_path = cached_path.normalize_with(main_field, self.cache.as_ref());
602+
// d. LOAD_AS_FILE(M)
603+
if let Some(path) = self.load_as_file(&cached_path, ctx)? {
604+
return Ok(Some(path));
605+
}
606+
// e. LOAD_INDEX(M)
607+
if let Some(path) = self.load_index(&cached_path, ctx)? {
608+
return Ok(Some(path));
613609
}
614-
// f. LOAD_INDEX(X) DEPRECATED
615-
// g. THROW "not found"
616610
}
611+
// f. LOAD_INDEX(X) DEPRECATED
612+
// g. THROW "not found"
617613
}
618614
// 2. LOAD_INDEX(X)
619615
self.load_index(cached_path, ctx)
@@ -1283,7 +1279,6 @@ impl<C: Cache> ResolverGeneric<C> {
12831279
Some(b'.') => Ok(tsconfig.directory().normalize_with(specifier)),
12841280
_ => self
12851281
.clone_with_options(ResolveOptions {
1286-
description_files: vec![],
12871282
extensions: vec![".json".into()],
12881283
main_files: vec!["tsconfig.json".into()],
12891284
..ResolveOptions::default()

src/options.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,6 @@ pub struct ResolveOptions {
4141
/// Default `[]`
4242
pub condition_names: Vec<String>,
4343

44-
/// The JSON files to use for descriptions. (There was once a `bower.json`.)
45-
///
46-
/// Default `["package.json"]`
47-
pub description_files: Vec<String>,
48-
4944
/// Set to [EnforceExtension::Enabled] for [ESM Mandatory file extensions](https://nodejs.org/api/esm.html#mandatory-file-extensions).
5045
///
5146
/// If `enforce_extension` is set to [EnforceExtension::Enabled], resolution will not allow extension-less files.
@@ -443,7 +438,6 @@ impl Default for ResolveOptions {
443438
alias: vec![],
444439
alias_fields: vec![],
445440
condition_names: vec![],
446-
description_files: vec!["package.json".into()],
447441
enforce_extension: EnforceExtension::Auto,
448442
extension_alias: vec![],
449443
exports_fields: vec![vec!["exports".into()]],
@@ -590,7 +584,6 @@ mod test {
590584
alias_fields: vec![],
591585
builtin_modules: false,
592586
condition_names: vec![],
593-
description_files: vec![],
594587
enforce_extension: EnforceExtension::Disabled,
595588
exports_fields: vec![],
596589
extension_alias: vec![],

src/tests/incorrect_description_file.rs

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
use rustc_hash::FxHashSet;
44

5-
use crate::{JSONError, Resolution, ResolveContext, ResolveError, ResolveOptions, Resolver};
5+
use crate::{JSONError, ResolveContext, ResolveError, Resolver};
66

77
// should not resolve main in incorrect description file #1
88
#[test]
@@ -45,21 +45,3 @@ fn incorrect_description_file_3() {
4545
let resolution = Resolver::default().resolve(f.join("pack2"), ".");
4646
assert!(resolution.is_err());
4747
}
48-
49-
// `enhanced_resolve` does not have this test case
50-
#[test]
51-
fn no_description_file() {
52-
let f = super::fixture_root().join("enhanced_resolve");
53-
54-
// has description file
55-
let resolver = Resolver::default();
56-
assert_eq!(
57-
resolver.resolve(&f, ".").map(Resolution::into_path_buf),
58-
Ok(f.join("lib/index.js"))
59-
);
60-
61-
// without description file
62-
let resolver =
63-
Resolver::new(ResolveOptions { description_files: vec![], ..ResolveOptions::default() });
64-
assert_eq!(resolver.resolve(&f, "."), Err(ResolveError::NotFound(".".into())));
65-
}

0 commit comments

Comments
 (0)