Skip to content

Commit 0bdd3f0

Browse files
authored
refactor: selectively parse package_json fields instead of parsing everything (#103)
so we don't end up parsing unwanted fields and produce errors we don't care about.
1 parent fa51208 commit 0bdd3f0

File tree

6 files changed

+83
-141
lines changed

6 files changed

+83
-141
lines changed

napi/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,6 @@ impl ResolverFactory {
157157
.unwrap_or(default.roots),
158158
symlinks: op.symlinks.unwrap_or(default.symlinks),
159159
builtin_modules: op.builtin_modules.unwrap_or(default.builtin_modules),
160-
remove_unused_fields: op.remove_unused_fields.unwrap_or(default.remove_unused_fields),
161160
}
162161
}
163162
}

napi/src/options.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,6 @@ pub struct NapiResolveOptions {
141141
///
142142
/// Default `false`
143143
pub builtin_modules: Option<bool>,
144-
145-
/// When enabled, will attempt to reduce memory usage by clearing data and fields
146-
/// that are not relevant to the current process, and should not be held in memory.
147-
pub remove_unused_fields: Option<bool>,
148144
}
149145

150146
#[napi]

src/lib.rs

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,9 @@ impl<Fs: FileSystem + Default> ResolverGeneric<Fs> {
440440
return Ok(None);
441441
};
442442
// 3. If the SCOPE/package.json "imports" is null or undefined, return.
443-
if package_json.imports.is_empty() {
443+
if package_json.imports.is_none()
444+
|| package_json.imports.as_ref().is_some_and(|imports| imports.is_empty())
445+
{
444446
return Ok(None);
445447
}
446448
// 4. let MATCH = PACKAGE_IMPORTS_RESOLVE(X, pathToFileURL(SCOPE), ["node", "require"]) defined in the ESM resolver.
@@ -753,7 +755,6 @@ impl<Fs: FileSystem + Default> ResolverGeneric<Fs> {
753755
if !package_json.exports.is_empty() {
754756
// 4. If the SCOPE/package.json "name" is not the first segment of X, return.
755757
if let Some(subpath) = package_json
756-
.data
757758
.name
758759
.as_ref()
759760
.and_then(|package_name| Self::strip_package_name(specifier, package_name))
@@ -1266,16 +1267,18 @@ impl<Fs: FileSystem + Default> ResolverGeneric<Fs> {
12661267
// 2. If pjson.imports is a non-null Object, then
12671268

12681269
// 1. Let resolved be the result of PACKAGE_IMPORTS_EXPORTS_RESOLVE( specifier, pjson.imports, packageURL, true, conditions).
1269-
if let Some(path) = self.package_imports_exports_resolve(
1270-
specifier,
1271-
&package_json.imports,
1272-
package_json.directory(),
1273-
/* is_imports */ true,
1274-
&self.options.condition_names,
1275-
ctx,
1276-
)? {
1277-
// 2. If resolved is not null or undefined, return resolved.
1278-
return Ok(path);
1270+
if let Some(imports) = &package_json.imports {
1271+
if let Some(path) = self.package_imports_exports_resolve(
1272+
specifier,
1273+
imports,
1274+
package_json.directory(),
1275+
/* is_imports */ true,
1276+
&self.options.condition_names,
1277+
ctx,
1278+
)? {
1279+
// 2. If resolved is not null or undefined, return resolved.
1280+
return Ok(path);
1281+
}
12791282
}
12801283

12811284
// 5. Throw a Package Import Not Defined error.

src/options.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,6 @@ pub struct ResolveOptions {
133133
///
134134
/// Default `false`
135135
pub builtin_modules: bool,
136-
137-
/// When enabled, will attempt to reduce memory usage by clearing data and fields
138-
/// that are not relevant to the current process, and should not be held in memory.
139-
pub remove_unused_fields: bool,
140136
}
141137

142138
impl ResolveOptions {
@@ -377,7 +373,6 @@ impl Default for ResolveOptions {
377373
roots: vec![],
378374
symlinks: true,
379375
builtin_modules: false,
380-
remove_unused_fields: true,
381376
}
382377
}
383378
}

src/package_json.rs

Lines changed: 65 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -6,58 +6,51 @@ use std::{
66
sync::Arc,
77
};
88

9-
use nodejs_package_json::{
10-
BrowserField, ImportExportField, ImportExportMap, PackageJson as BasePackageJson,
11-
};
12-
use rustc_hash::FxHashMap;
9+
use nodejs_package_json::{BrowserField, ImportExportField, ImportExportMap};
1310
use serde::Deserialize;
1411
use serde_json::Value;
1512

1613
use crate::{path::PathUtil, ResolveError, ResolveOptions};
1714

1815
/// Deserialized package.json
19-
#[derive(Debug, Default, Deserialize)]
16+
#[derive(Debug, Default)]
2017
pub struct PackageJson {
2118
/// Path to `package.json`. Contains the `package.json` filename.
22-
#[serde(skip)]
2319
pub path: PathBuf,
2420

2521
/// Realpath to `package.json`. Contains the `package.json` filename.
26-
#[serde(skip)]
2722
pub realpath: PathBuf,
2823

29-
#[serde(skip)]
3024
pub(crate) raw_json: Arc<serde_json::Value>,
3125

32-
/// The deserialized `package.json`.
33-
pub data: BasePackageJson,
26+
/// The "name" field defines your package's name.
27+
/// The "name" field can be used in addition to the "exports" field to self-reference a package using its name.
28+
///
29+
/// <https://nodejs.org/api/packages.html#name>
30+
pub name: Option<String>,
3431

3532
/// The "main" field defines the entry point of a package when imported by name via a node_modules lookup. Its value is a path.
3633
/// When a package has an "exports" field, this will take precedence over the "main" field when importing the package by name.
3734
///
3835
/// Values are dynamically added from [ResolveOptions::main_fields].
3936
///
4037
/// <https://nodejs.org/api/packages.html#main>
41-
#[serde(skip)]
4238
pub main_fields: Vec<String>,
4339

4440
/// The "exports" field allows defining the entry points of a package when imported by name loaded either via a node_modules lookup or a self-reference to its own name.
4541
///
4642
/// <https://nodejs.org/api/packages.html#exports>
47-
#[serde(skip)]
4843
pub exports: Vec<ImportExportField>,
4944

5045
/// In addition to the "exports" field, there is a package "imports" field to create private mappings that only apply to import specifiers from within the package itself.
5146
///
5247
/// <https://nodejs.org/api/packages.html#subpath-imports>
53-
#[serde(default)]
54-
pub imports: Box<ImportExportMap>,
48+
pub imports: Option<Box<ImportExportMap>>,
5549

5650
/// The "browser" field is provided by a module author as a hint to javascript bundlers or component tools when packaging modules for client side use.
5751
/// Multiple values are configured by [ResolveOptions::alias_fields].
5852
///
5953
/// <https://github.com/defunctzombie/package-browser-field-spec>
60-
#[serde(skip)]
6154
pub browser_fields: Vec<BrowserField>,
6255
}
6356

@@ -71,127 +64,85 @@ impl PackageJson {
7164
options: &ResolveOptions,
7265
) -> Result<Self, serde_json::Error> {
7366
let mut raw_json: Value = serde_json::from_str(json)?;
74-
let mut data: BasePackageJson = serde_json::from_value(raw_json.clone())?;
75-
7667
let mut package_json = Self::default();
68+
7769
package_json.main_fields.reserve_exact(options.main_fields.len());
78-
package_json.browser_fields.reserve_exact(options.alias_fields.len());
7970
package_json.exports.reserve_exact(options.exports_fields.len());
71+
package_json.browser_fields.reserve_exact(options.alias_fields.len());
8072

81-
// Dynamically create `main_fields`.
82-
for main_field_key in &options.main_fields {
83-
match main_field_key.as_str() {
84-
"main" => {
85-
if let Some(main_field) = &data.main {
86-
package_json.main_fields.push(main_field.to_string_lossy().to_string());
87-
}
88-
}
89-
"module" => {
90-
if let Some(module_field) = &data.module {
91-
package_json.main_fields.push(module_field.to_string_lossy().to_string());
92-
}
93-
}
94-
"browser" => {
95-
if let Some(BrowserField::String(browser_field)) = &data.browser {
96-
package_json.main_fields.push(browser_field.to_owned());
97-
}
98-
}
99-
field => {
100-
// Using `get` + `clone` instead of remove here
101-
// because `main_fields` may contain `browser`, which is also used in `browser_fields.
102-
if let Some(Value::String(value)) = data.other_fields.get(field) {
103-
package_json.main_fields.push(value.clone());
104-
}
105-
}
106-
};
107-
}
108-
109-
// Dynamically create `browser_fields`.
110-
let dir = path.parent().unwrap();
111-
for object_path in &options.alias_fields {
112-
let mut browser_field = if object_path.len() == 1 && object_path[0] == "browser" {
113-
if let Some(field) = &data.browser {
114-
field.to_owned()
115-
} else {
116-
continue;
73+
if let Some(json_object) = raw_json.as_object_mut() {
74+
// Remove large fields that are useless for pragmatic use.
75+
json_object.remove("description");
76+
json_object.remove("keywords");
77+
json_object.remove("scripts");
78+
json_object.remove("dependencies");
79+
json_object.remove("devDependencies");
80+
json_object.remove("peerDependencies");
81+
json_object.remove("optionalDependencies");
82+
83+
// Add name.
84+
package_json.name =
85+
json_object.get("name").and_then(|field| field.as_str()).map(ToString::to_string);
86+
87+
// Add imports.
88+
package_json.imports = json_object
89+
.get("imports")
90+
.map(ImportExportMap::deserialize)
91+
.transpose()?
92+
.map(Box::new);
93+
94+
// Dynamically create `main_fields`.
95+
for main_field_key in &options.main_fields {
96+
// Using `get` + `clone` instead of remove here
97+
// because `main_fields` may contain `browser`, which is also used in `browser_fields.
98+
if let Some(serde_json::Value::String(value)) = json_object.get(main_field_key) {
99+
package_json.main_fields.push(value.clone());
117100
}
118-
} else if let Some(field) = Self::get_value_by_path(&data.other_fields, object_path) {
119-
BrowserField::deserialize(field)?
120-
} else {
121-
continue;
122-
};
101+
}
123102

124-
// Normalize all relative paths to make browser_field a constant value lookup
125-
if let BrowserField::Map(map) = &mut browser_field {
126-
let keys = map.keys().cloned().collect::<Vec<_>>();
127-
for key in keys {
128-
// Normalize the key if it looks like a file "foo.js"
129-
if key.extension().is_some() {
130-
map.insert(dir.normalize_with(&key), map[&key].clone());
131-
}
132-
// Normalize the key if it is relative path "./relative"
133-
if key.starts_with(".") {
134-
if let Some(value) = map.remove(&key) {
135-
map.insert(dir.normalize_with(&key), value);
103+
// Dynamically create `browser_fields`.
104+
let dir = path.parent().unwrap();
105+
for object_path in &options.alias_fields {
106+
if let Some(browser_field) = Self::get_value_by_path(json_object, object_path) {
107+
let mut browser_field = BrowserField::deserialize(browser_field)?;
108+
109+
// Normalize all relative paths to make browser_field a constant value lookup
110+
if let BrowserField::Map(map) = &mut browser_field {
111+
let keys = map.keys().cloned().collect::<Vec<_>>();
112+
for key in keys {
113+
// Normalize the key if it looks like a file "foo.js"
114+
if key.extension().is_some() {
115+
map.insert(dir.normalize_with(&key), map[&key].clone());
116+
}
117+
// Normalize the key if it is relative path "./relative"
118+
if key.starts_with(".") {
119+
if let Some(value) = map.remove(&key) {
120+
map.insert(dir.normalize_with(&key), value);
121+
}
122+
}
136123
}
137124
}
125+
package_json.browser_fields.push(browser_field);
138126
}
139127
}
140-
package_json.browser_fields.push(browser_field);
141-
}
142128

143-
// Dynamically create `exports`.
144-
for object_path in &options.exports_fields {
145-
if object_path.len() == 1 && object_path[0] == "exports" {
146-
if let Some(exports) = &data.exports {
147-
package_json.exports.push(exports.to_owned());
129+
// Dynamically create `exports`.
130+
for object_path in &options.exports_fields {
131+
if let Some(exports) = Self::get_value_by_path(json_object, object_path) {
132+
let exports = ImportExportField::deserialize(exports)?;
133+
package_json.exports.push(exports);
148134
}
149-
} else if let Some(exports) = Self::get_value_by_path(&data.other_fields, object_path) {
150-
package_json.exports.push(ImportExportField::deserialize(exports)?);
151135
}
152136
}
153137

154-
// Bubble up `imports`
155-
if let Some(imports) = &data.imports {
156-
package_json.imports = Box::new(imports.to_owned());
157-
}
158-
159138
package_json.path = path;
160139
package_json.realpath = realpath;
161-
162-
// Remove large fields that are useless for pragmatic use
163-
if options.remove_unused_fields {
164-
data.scripts = None;
165-
data.dependencies = None;
166-
data.dependencies_meta = None;
167-
data.dev_dependencies = None;
168-
data.peer_dependencies = None;
169-
data.peer_dependencies_meta = None;
170-
data.bundle_dependencies = None;
171-
data.optional_dependencies = None;
172-
data.imports = None;
173-
data.exports = None;
174-
data.browser = None;
175-
176-
if let Some(raw_json) = raw_json.as_object_mut() {
177-
raw_json.remove("description");
178-
raw_json.remove("keywords");
179-
raw_json.remove("scripts");
180-
raw_json.remove("dependencies");
181-
raw_json.remove("devDependencies");
182-
raw_json.remove("peerDependencies");
183-
raw_json.remove("optionalDependencies");
184-
}
185-
}
186-
187-
package_json.data = data;
188140
package_json.raw_json = Arc::new(raw_json);
189-
190141
Ok(package_json)
191142
}
192143

193144
fn get_value_by_path<'a>(
194-
fields: &'a FxHashMap<String, serde_json::Value>,
145+
fields: &'a serde_json::Map<String, serde_json::Value>,
195146
path: &[String],
196147
) -> Option<&'a serde_json::Value> {
197148
if path.is_empty() {

tests/integration_test.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,9 @@ fn eq() {
3535
#[test]
3636
fn package_json() {
3737
let resolution = resolve("./tests/package.json");
38-
assert!(resolution.package_json().is_some_and(|json| json
39-
.data
40-
.name
41-
.as_ref()
42-
.is_some_and(|name| name == "name")));
38+
assert!(resolution
39+
.package_json()
40+
.is_some_and(|json| json.name.as_ref().is_some_and(|name| name == "name")));
4341
}
4442

4543
#[test]

0 commit comments

Comments
 (0)