-
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(repl): improve package.json support #18497
Conversation
Thanks for the quick fix! Does this mean that REPL will still look for package.json even before there's an import? Why does it need to do that? |
Yes, it will still do that. It's because the repl internally uses the lsp and the lsp eagerly loads it. The lsp eagerly loads it in order to have consistent/deterministic npm resolution regardless of what files are currently open or loaded. |
Thinking about it more, we could maybe only use the lockfile as a source of truth for deterministic resolution. I will look into changing this. Edit: This is now resolved. |
@@ -106,7 +106,7 @@ pub fn result_to_evaluation_output( | |||
match r { | |||
Ok(value) => value, | |||
Err(err) => { | |||
EvaluationOutput::Error(format!("{} {}", colors::red("error:"), err)) | |||
EvaluationOutput::Error(format!("{} {:#}", colors::red("error:"), err)) |
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.
Outputs the error with context.
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, good fix!
super::logging::set_lsp_log_level(log::Level::Debug); | ||
super::logging::set_lsp_warn_level(log::Level::Debug); |
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.
Elegant solution 👍
Closes #17929
Closes #18494