Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: regard file extensions during path resolution (#133) #193

Merged
merged 1 commit into from
Mar 3, 2022

Conversation

aleclarson
Copy link
Contributor

Originally during the file path resolution, file extensions were removed
without explicit reason. This commit changes the resolution logic to keep file
extensions, with the goal to add support for modern non-js extensions
like cjs. The change however keeps /xyz/index resolves as is as they
still should resolve to /xyz.

Refs 847d314

Originally during the file path resolution, file extensions were removed
without explicit reason. This commit changes the resolution logic to keep file
extensions, with the goal to add support for modern non-js extensions
like cjs. The change however keeps /xyz/index resolves as is as they
still should resolve to /xyz.

Refs 847d314

Co-authored-by: Andrew Bradley <[email protected]>
Co-authored-by: Alec Larson <[email protected]>
@aleclarson aleclarson force-pushed the fix-cjs-main-requires branch from 8e25064 to 95b88f3 Compare March 3, 2022 21:42
@aleclarson aleclarson mentioned this pull request Mar 3, 2022
@jonaskello
Copy link
Member

Thanks! :-)

@jonaskello
Copy link
Member

Released in 3.13.0

@JiangWeixian
Copy link

JiangWeixian commented Apr 5, 2023

Just curious about why remove extensions from path...

// Not sure why we don't just return the full found path?
export function getStrippedPath(tryPath: TryPath): string {
return tryPath.type === "index"
? dirname(tryPath.path)
: tryPath.type === "file"
? tryPath.path
: tryPath.type === "extension"
? removeExtension(tryPath.path)
: tryPath.type === "package"
? tryPath.path
: exhaustiveTypeException(tryPath.type);
}

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.

4 participants