-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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(node): show directory import and missing extension suggestions #27905
base: main
Are you sure you want to change the base?
fix(node): show directory import and missing extension suggestions #27905
Conversation
@@ -143,6 +144,39 @@ impl NodeResolution { | |||
} | |||
} | |||
|
|||
struct LocalPath { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming is hard.
} | ||
} | ||
LocalUrlOrPath::Path(LocalPath { path, known_exists }) => { | ||
if known_exists { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As part of this change, we do the finalize_resolution step when resolving npm specifiers too (so that these better errors can happen and so the node resolution code is always using the same path). I was concerened about the extra fs hit that brings, so I updated the code to also keep track of whether we know a path exists in order to avoid the finalization check if the path is known to exist.
Would it be okay to close the below PR due to this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, very useful hint
Shows directory import and missing extension suggestions in error messages similar but not exactly to node.