-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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(check): compiler options from workspace members #27009
base: main
Are you sure you want to change the base?
fix(check): compiler options from workspace members #27009
Conversation
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.
TODO: Add needed deno_config APIs, currently using hacks but it works.
Considering the 2.1 release passed, maybe let's spend the time to do this right before merging?
Yeah this is not for landing yet |
…ber-compiler-options
…ber-compiler-options
…ber-compiler-options
…ber-compiler-options
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.
Does this work for deno test
and deno bench
?
} else { | ||
found_specifiers = true; | ||
} | ||
let factory = CliFactory::from_cli_options(Arc::new(cli_options)); |
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.
Ideally we should only be creating a single factory. Why is all this setup code necessary here?
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.
The factory's services used below are based on a single WorkspaceDirectory
currently. I wanted to contain the changes somewhere for now but I can keep working on it.
ConfigFlag::Path(_) => false, | ||
ConfigFlag::Disabled => false, | ||
}; | ||
if is_discovered_config { |
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.
Why do we only do this for discovered configuration files? Can you add a comment explaining?
.await | ||
Ok( | ||
main_graph_container | ||
.check_specifiers(&specifiers_for_typecheck, None) |
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.
Maybe we should handle this exclusively in check_specifiers
?
…ber-compiler-options
Closes #24504.
Blocked by denoland/deno_config#143.
Blocked by crate updates in #27096.