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

refactor(node_resolver): make conditions_from_resolution_mode configurable #27596

Merged
merged 34 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
31e16e2
added lib
theswerd Nov 18, 2024
35c627a
Merge branch 'denoland:main' into main
theswerd Dec 24, 2024
d7ffef8
Merge branch 'main' of https://github.com/freestyle-sh/deno
theswerd Jan 9, 2025
b925228
made conditions_from_resolution_mode configurable
theswerd Jan 9, 2025
d21a902
re-forcing ci
theswerd Jan 9, 2025
fc0f5d6
re-forcing ci
theswerd Jan 9, 2025
7b25307
ooop slight syntax sugar
theswerd Jan 9, 2025
460fcaa
will this work maybe?
theswerd Jan 9, 2025
0188efc
ok now we have a struct with debug so that fixes that + its arced so …
theswerd Jan 9, 2025
fdf915a
force debug
theswerd Jan 9, 2025
555fae3
function pub
theswerd Jan 9, 2025
56edca4
how about htis
theswerd Jan 9, 2025
a081d2c
resolution arc fix
theswerd Jan 9, 2025
86ca545
ooops
theswerd Jan 9, 2025
656da92
ok could this work
theswerd Jan 9, 2025
bc1fcd1
ok its formatted will it work now please dear lord
theswerd Jan 9, 2025
91fef55
ok this could be it please dear lord
theswerd Jan 9, 2025
6d491b6
resolution
theswerd Jan 9, 2025
3a06e2c
arc maybe
theswerd Jan 9, 2025
ba4f7b2
ok now its send and sync
theswerd Jan 9, 2025
a686645
ooof baboof
theswerd Jan 9, 2025
fbd4b63
ooof again
theswerd Jan 9, 2025
0a2fda4
re-forcing ci
theswerd Jan 9, 2025
ec8447c
ok made the change
theswerd Jan 9, 2025
deb26e2
ooops lint fix
theswerd Jan 9, 2025
8797417
ok we're going well
theswerd Jan 9, 2025
ee8aae7
Merge branch 'main' into conditions_from_resolution_mode
theswerd Jan 10, 2025
f513538
ok, made requested changes
theswerd Jan 10, 2025
12f061a
default
theswerd Jan 10, 2025
2a1baad
Merge branch 'main' into conditions_from_resolution_mode
theswerd Jan 10, 2025
1706cab
fix lint issue
theswerd Jan 10, 2025
f39faeb
Merge remote-tracking branch 'refs/remotes/origin/conditions_from_res…
theswerd Jan 10, 2025
7ee74bd
APplied patch
theswerd Jan 10, 2025
e0e3338
Merge branch 'main' into conditions_from_resolution_mode
theswerd Jan 11, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,9 @@ impl CliFactory {
.into_npm_pkg_folder_resolver(),
self.pkg_json_resolver().clone(),
self.sys(),
node_resolver::ConditionsFromResolutionMode::new(
node_resolver::deno_conditions_from_resolution_mode,
),
)))
}
.boxed_local(),
Expand Down
3 changes: 3 additions & 0 deletions cli/lsp/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,9 @@ impl<'a> ResolverFactory<'a> {
npm_resolver.clone().into_npm_pkg_folder_resolver(),
self.pkg_json_resolver.clone(),
self.sys.clone(),
node_resolver::ConditionsFromResolutionMode::new(
node_resolver::deno_conditions_from_resolution_mode,
),
)))
})
.as_ref()
Expand Down
3 changes: 3 additions & 0 deletions cli/standalone/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,9 @@ pub async fn run(
npm_resolver.clone().into_npm_pkg_folder_resolver(),
pkg_json_resolver.clone(),
sys.clone(),
node_resolver::ConditionsFromResolutionMode::new(
node_resolver::deno_conditions_from_resolution_mode,
),
));
let cjs_tracker = Arc::new(CjsTracker::new(
in_npm_pkg_checker.clone(),
Expand Down
3 changes: 3 additions & 0 deletions resolvers/node/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ mod npm;
mod package_json;
mod path;
mod resolution;

mod sync;

pub use deno_package_json::PackageJson;
Expand All @@ -20,8 +21,10 @@ pub use package_json::PackageJsonResolver;
pub use package_json::PackageJsonResolverRc;
pub use package_json::PackageJsonThreadLocalCache;
pub use path::PathClean;
pub use resolution::deno_conditions_from_resolution_mode;
pub use resolution::parse_npm_pkg_name;
pub use resolution::resolve_specifier_into_node_modules;
pub use resolution::ConditionsFromResolutionMode;
pub use resolution::IsBuiltInNodeModuleChecker;
pub use resolution::NodeResolution;
pub use resolution::NodeResolutionKind;
Expand Down
38 changes: 34 additions & 4 deletions resolvers/node/resolution.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2018-2025 the Deno authors. MIT license.

use std::borrow::Cow;
use std::fmt::Debug;
use std::path::Path;
use std::path::PathBuf;

Expand Down Expand Up @@ -46,6 +47,8 @@ use crate::errors::TypesNotFoundErrorData;
use crate::errors::UnsupportedDirImportError;
use crate::errors::UnsupportedEsmUrlSchemeError;
use crate::npm::InNpmPackageCheckerRc;
#[allow(clippy::disallowed_types)]
use crate::sync::MaybeArc;
use crate::NpmPackageFolderResolverRc;
use crate::PackageJsonResolverRc;
use crate::PathClean;
Expand All @@ -54,7 +57,7 @@ pub static DEFAULT_CONDITIONS: &[&str] = &["deno", "node", "import"];
pub static REQUIRE_CONDITIONS: &[&str] = &["require", "node"];
static TYPES_ONLY_CONDITIONS: &[&str] = &["types"];

fn conditions_from_resolution_mode(
pub fn deno_conditions_from_resolution_mode(
resolution_mode: ResolutionMode,
) -> &'static [&'static str] {
match resolution_mode {
Expand All @@ -63,6 +66,30 @@ fn conditions_from_resolution_mode(
}
}

pub struct ConditionsFromResolutionMode {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should just be: pub struct ConditionsFromResolutionMode(Option<Box<dyn Fn(ResolutionMode) -> &'static [&'static str]>>) with a default implementation of ConditionsFromResolutionMode(None) that will instead use deno_conditions_from_resolution_mode? That way the other code can just do ConditionsFromResolutionMode::default() instead of needing to know about deno_conditions_from_resolution_mode

#[allow(clippy::disallowed_types)]
pub func: MaybeArc<
dyn Fn(ResolutionMode) -> &'static [&'static str] + Send + Sync + 'static,
>,
}

impl Debug for ConditionsFromResolutionMode {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("ConditionsFromResolutionMode").finish()
}
}

impl ConditionsFromResolutionMode {
pub fn new(
func: impl Fn(ResolutionMode) -> &'static [&'static str] + Send + Sync + 'static,
) -> Self {
Self {
#[allow(clippy::disallowed_types)]
func: MaybeArc::new(func),
}
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
pub enum ResolutionMode {
Import,
Expand Down Expand Up @@ -120,6 +147,7 @@ pub struct NodeResolver<
npm_pkg_folder_resolver: NpmPackageFolderResolverRc,
pkg_json_resolver: PackageJsonResolverRc<TSys>,
sys: TSys,
conditions_from_resolution_mode: ConditionsFromResolutionMode,
}

impl<
Expand All @@ -133,13 +161,15 @@ impl<
npm_pkg_folder_resolver: NpmPackageFolderResolverRc,
pkg_json_resolver: PackageJsonResolverRc<TSys>,
sys: TSys,
conditions_from_resolution_mode: ConditionsFromResolutionMode,
) -> Self {
Self {
in_npm_pkg_checker,
is_built_in_node_module_checker,
npm_pkg_folder_resolver,
pkg_json_resolver,
sys,
conditions_from_resolution_mode,
}
}

Expand Down Expand Up @@ -201,7 +231,7 @@ impl<
specifier,
referrer,
resolution_mode,
conditions_from_resolution_mode(resolution_mode),
(self.conditions_from_resolution_mode.func)(resolution_mode),
resolution_kind,
)?;

Expand Down Expand Up @@ -343,7 +373,7 @@ impl<
&package_subpath,
maybe_referrer,
resolution_mode,
conditions_from_resolution_mode(resolution_mode),
(self.conditions_from_resolution_mode.func)(resolution_mode),
resolution_kind,
)?;
// TODO(bartlomieju): skipped checking errors for commonJS resolution and
Expand Down Expand Up @@ -474,7 +504,7 @@ impl<
/* sub path */ ".",
maybe_referrer,
resolution_mode,
conditions_from_resolution_mode(resolution_mode),
(self.conditions_from_resolution_mode.func)(resolution_mode),
NodeResolutionKind::Types,
);
if let Ok(resolution) = resolution_result {
Expand Down
Loading