-
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
refactor: add WorkspaceFactory
and ResolverFactory
#27766
Conversation
… needing other services to be constructed
…_resolver_factory
pub struct ImportMapSpecifierResolveError { | ||
#[source] | ||
source: deno_path_util::ResolveUrlOrPathError, | ||
} |
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.
I shifted this code out of the CLI, but then I moved it back. I kept the changes.
230d821
to
5afb334
Compare
self.options.no_npm | ||
} | ||
|
||
pub fn node_modules_dir_mode( |
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.
Pay attention here. I rewrote the logic for determining the NodeModulesDirMode because previously it was convoluted. This should be much more straightforward.
resolvers/deno/npmrc.rs
Outdated
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.
Extracted from CLI crate.
#[inline(always)] | ||
fn is_builtin_node_module(&self, module_name: &str) -> bool { | ||
DENO_SUPPORTED_BUILTIN_NODE_MODULES | ||
.binary_search(&module_name) |
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.
Changed this to binary search instead of iterating because DENO_SUPPORTED_BUILTIN_NODE_MODULES
is guaranteed to be sorted.
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.
Looks fine for me, but probably good to get another pair of eyes on it before landing.
/// Discover `.npmrc` file - currently we only support it next to `package.json`, | ||
/// next to `deno.json`, or in the user's home directory. | ||
/// | ||
/// In the future we will need to support it in the global directory | ||
/// as per https://docs.npmjs.com/cli/v10/configuring-npm/npmrc#files. | ||
pub fn discover_npmrc_from_workspace<TSys: EnvVar + EnvHomeDir + FsRead>( |
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.
This will cause conflicts with #27119 :(
pub static DENO_SUPPORTED_BUILTIN_NODE_MODULES: &[&str] = &[ | ||
// NOTE(bartlomieju): keep this list in sync with `ext/node/polyfills/01_require.js` |
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.
Make sure this is reachable when a crate is imported - Deploy relies on this list
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.
Double checked and it is.
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!
Allows easily constructing a `DenoResolver` using the exact same logic that we use in the CLI (useful for dnt and for external bundlers). This code is then used in the CLI to ensure the logic is always up-to-date. ```rs use std::rc::Rc; use deno_resolver::factory::ResolverFactory; use deno_resolver::factory::WorkspaceFactory; use sys_traits::impls::RealSys; let sys = RealSys; let cwd = sys.env_current_dir()?; let workspace_factory = Rc::new(WorkspaceFactory::new(sys, cwd, Default::default())); let resolver_factory = ResolverFactory::new(workspace_factory.clone(), Default::default()); let deno_resolver = resolver_factory.deno_resolver().await?; ```
Allows easily constructing a
DenoResolver
using the exact same logic that we use in the CLI (useful for dnt and for external bundlers). This code is then used in the CLI to ensure the logic is always up-to-date.