Skip to content

Revert "TURBOPACK: Fix subpath imports resolving to dependencies"#151

Merged
fireairforce merged 1 commit into
utoofrom
revert-150-fix/turbopack-subpath-imports-dependencies
May 21, 2026
Merged

Revert "TURBOPACK: Fix subpath imports resolving to dependencies"#151
fireairforce merged 1 commit into
utoofrom
revert-150-fix/turbopack-subpath-imports-dependencies

Conversation

@yuzheng14
Copy link
Copy Markdown

Reverts #150

Upstream has merged

@fireairforce fireairforce merged commit 450220a into utoo May 21, 2026
14 of 27 checks passed
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request simplifies the resolution logic in turbopack-core by removing the ExportImport enum and its associated conditional logic in handle_exports_imports_field, which streamlines the request parsing process. Additionally, the dep-in-nm dependency and its corresponding test cases have been removed. Feedback was provided to optimize the code by removing an unnecessary .clone() call on result_path when constructing the Pattern::Concatenation vector, as the variable is not used subsequently.

Comment on lines +3266 to +3271
let request = *Request::parse(Pattern::Concatenation(vec![
Pattern::Constant(rcstr!("./")),
result_path.clone(),
]))
.to_resolved()
.await?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The result_path variable is not used after being moved into the Pattern::Concatenation vector. Since it is an owned Pattern (returned by with_normalized_path), calling .clone() is unnecessary and incurs an extra allocation. You can move it directly.

            let request = *Request::parse(Pattern::Concatenation(vec![
                Pattern::Constant(rcstr!("./")),
                result_path,
            ]))
            .to_resolved()
            .await?;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants