Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,12 +167,12 @@ The options are aligned with [enhanced-resolve](https://github.com/webpack/enhan

### Other Options

| Field | Default | Description |
| ------------------- | ------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| tsconfig | None | TypeScript related config for resolver |
| tsconfig.configFile | | A relative path to the tsconfig file based on `cwd`, or an absolute path of tsconfig file. |
| tsconfig.references | `[]` | - 'auto': inherits from TypeScript config <br/> - `string []`: relative path (based on directory of the referencing tsconfig file) or absolute path of referenced project's tsconfig |
| enablePnp | false | Enable Yarn Plug'n'Play support |
| Field | Default | Description |
| ------------------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| tsconfig | None | TypeScript related config for resolver |
| tsconfig.configFile | | A relative path to the tsconfig file based on `cwd`, or an absolute path of tsconfig file. |
| tsconfig.references | `[]` | - 'auto': inherits from TypeScript config <br/> - `string []`: relative path (based on directory of the referencing tsconfig file) or absolute path of referenced project's tsconfig |
| enablePnp | false | Enable Yarn Plug'n'Play support |

In the context of `@rspack/resolver`, the `tsconfig.references` option helps isolate the `paths` configurations of different TypeScript projects.
This ensures that path aliases defined in one TypeScript project do not unintentionally affect the resolving behavior of another.
Expand Down
8 changes: 7 additions & 1 deletion src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl<Fs: Send + Sync + FileSystem> Cache<Fs> {
}
}

#[derive(Clone)]
#[derive(Clone, Debug)]
pub struct CachedPath(Arc<CachedPathImpl>);

impl Hash for CachedPath {
Expand Down Expand Up @@ -158,6 +158,12 @@ pub struct CachedPathImpl {
package_json: OnceLock<Option<Arc<PackageJson>>>,
}

impl std::fmt::Debug for CachedPathImpl {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.path.fmt(f)
}
}

impl CachedPathImpl {
fn new(hash: u64, path: Box<Path>, parent: Option<CachedPath>) -> Self {
Self {
Expand Down
25 changes: 16 additions & 9 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1948,7 +1948,8 @@ impl<Fs: FileSystem + Send + Sync> ResolverGeneric<Fs> {
));
}
// 2. For each item targetValue in target, do
for (i, target_value) in targets.iter().enumerate() {
let mut last_error = None;
for target_value in targets.iter() {
// 1. Let resolved be the result of PACKAGE_TARGET_RESOLVE( packageURL, targetValue, patternMatch, isImports, conditions), continuing the loop on any Invalid Package Target error.
let resolved = self
.package_target_resolve(
Expand All @@ -1962,18 +1963,24 @@ impl<Fs: FileSystem + Send + Sync> ResolverGeneric<Fs> {
)
.await;

if resolved.is_err() && i == targets.len() {
return resolved;
}

// 2. If resolved is undefined, continue the loop.
if let Ok(Some(path)) = resolved {
match resolved {
// Only track InvalidPackageTarget for re-throwing after the loop.
Err(e @ ResolveError::InvalidPackageTarget(..)) => {
last_error = Some(e);
}
Err(_) => {}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Propagate non-InvalidPackageTarget fallback errors

This Err(_) => {} arm drops later fallback errors even though the array algorithm is only supposed to continue on InvalidPackageTarget. A config like {"./foo/": ["invalid", "./bar"]} resolving pkg/foo/x makes the second entry fail with InvalidPackageConfigDirectory in normalize_string_target, but this loop ignores that and rethrows the stale first InvalidPackageTarget from last_error. That changes the surfaced error away from the entry that actually failed and makes misconfigured fallback arrays much harder to diagnose.

Useful? React with 👍 / 👎.

// 2. If resolved is undefined, continue the loop.
Ok(None) => {
last_error = None;
}
// 3. Return resolved.
return Ok(Some(path));
Ok(Some(path)) => return Ok(Some(path)),
}
Comment on lines +1966 to 1978
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

While this change correctly handles InvalidPackageTarget errors in a fallback array, the Err(_) => {} case will silently swallow any other type of ResolveError. This could hide other resolution issues. According to the Node.js spec and enhanced-resolve's behavior, only InvalidPackageTarget errors should allow the loop to continue. Other errors should be propagated immediately.

            match resolved {
              // Only track InvalidPackageTarget for re-throwing after the loop.
              Err(e @ ResolveError::InvalidPackageTarget(..)) => {
                last_error = Some(e);
              }
              // Other errors should be propagated immediately.
              Err(e) => return Err(e),
              // 2. If resolved is undefined, continue the loop.
              Ok(None) => {
                last_error = None;
              }
              // 3. Return resolved.
              Ok(Some(path)) => return Ok(Some(path)),
            }

}
// 3. Return or throw the last fallback resolution null return or error.
// Note: see `resolved.is_err() && i == targets.len()`
if let Some(e) = last_error {
return Err(e);
}
}
JSONValue::Static(_) => {}
}
Expand Down
23 changes: 23 additions & 0 deletions src/tests/exports_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2634,3 +2634,26 @@ async fn test_cases() {
}
}
}

/// When all targets in a fallback array are invalid, `package_target_resolve` should
/// propagate `InvalidPackageTarget` instead of swallowing it and returning `Ok(None)`.
/// Bug: `resolved.is_err() && i == targets.len()` is always false (off-by-one),
/// causing errors to be silently dropped.
#[tokio::test]
async fn array_fallback_all_invalid_targets_should_return_invalid_package_target() {
let resolved = Resolver::new(ResolveOptions::default())
.package_exports_resolve(
Path::new(""),
".",
&exports_field(json!({
".": ["invalid_target_1", "invalid_target_2"]
})),
&mut Ctx::default(),
)
.await;

assert!(
matches!(resolved, Err(ResolveError::InvalidPackageTarget(..))),
"expected InvalidPackageTarget, got {resolved:?}"
);
}
Loading