fix(npm): improve package.json exports support for types (#16880)

This commit is contained in:
David Sherret 2022-11-30 18:07:32 -05:00 committed by GitHub
parent d1962e07af
commit b1e29d1bd0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 148 additions and 70 deletions

View file

@ -14,7 +14,6 @@ use crate::file_fetcher::SUPPORTED_SCHEMES;
use crate::node;
use crate::node::node_resolve_npm_reference;
use crate::node::NodeResolution;
use crate::node::NodeResolutionMode;
use crate::npm::NpmPackageReference;
use crate::npm::NpmPackageReq;
use crate::npm::NpmPackageResolver;
@ -33,6 +32,7 @@ use deno_core::url;
use deno_core::ModuleSpecifier;
use deno_graph::GraphImport;
use deno_graph::Resolved;
use deno_runtime::deno_node::NodeResolutionMode;
use once_cell::sync::Lazy;
use std::collections::BTreeMap;
use std::collections::HashMap;
@ -957,7 +957,7 @@ impl Documents {
node::node_resolve(
&specifier,
referrer,
node::NodeResolutionMode::Types,
NodeResolutionMode::Types,
npm_resolver,
)
.ok()

View file

@ -27,12 +27,12 @@ use deno_runtime::deno_node::package_imports_resolve;
use deno_runtime::deno_node::package_resolve;
use deno_runtime::deno_node::path_to_declaration_path;
use deno_runtime::deno_node::NodeModuleKind;
use deno_runtime::deno_node::NodeResolutionMode;
use deno_runtime::deno_node::PackageJson;
use deno_runtime::deno_node::PathClean;
use deno_runtime::deno_node::RequireNpmResolver;
use deno_runtime::deno_node::DEFAULT_CONDITIONS;
use deno_runtime::deno_node::NODE_GLOBAL_THIS_NAME;
use deno_runtime::deno_node::TYPES_CONDITIONS;
use once_cell::sync::Lazy;
use regex::Regex;
@ -108,12 +108,6 @@ impl NodeResolution {
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum NodeResolutionMode {
Execution,
Types,
}
struct NodeModulePolyfill {
/// Name of the module like "assert" or "timers/promises"
name: &'static str,
@ -480,8 +474,13 @@ pub fn node_resolve(
}
}
let conditions = mode_conditions(mode);
let url = module_resolve(specifier, referrer, conditions, npm_resolver)?;
let url = module_resolve(
specifier,
referrer,
DEFAULT_CONDITIONS,
mode,
npm_resolver,
)?;
let url = match url {
Some(url) => url,
None => return Ok(None),
@ -522,7 +521,8 @@ pub fn node_resolve_npm_reference(
.unwrap_or_else(|| ".".to_string()),
&package_folder,
node_module_kind,
mode_conditions(mode),
DEFAULT_CONDITIONS,
mode,
npm_resolver,
)
.with_context(|| {
@ -548,13 +548,6 @@ pub fn node_resolve_npm_reference(
Ok(Some(resolve_response))
}
fn mode_conditions(mode: NodeResolutionMode) -> &'static [&'static str] {
match mode {
NodeResolutionMode::Execution => DEFAULT_CONDITIONS,
NodeResolutionMode::Types => TYPES_CONDITIONS,
}
}
pub fn node_resolve_binary_export(
pkg_req: &NpmPackageReq,
bin_name: Option<&str>,
@ -669,6 +662,7 @@ fn package_config_resolve(
package_dir: &Path,
referrer_kind: NodeModuleKind,
conditions: &[&str],
mode: NodeResolutionMode,
npm_resolver: &dyn RequireNpmResolver,
) -> Result<Option<PathBuf>, AnyError> {
let package_json_path = package_dir.join("package.json");
@ -683,15 +677,15 @@ fn package_config_resolve(
&referrer,
referrer_kind,
conditions,
mode,
npm_resolver,
);
match result {
Ok(found) => return Ok(Some(found)),
Err(exports_err) => {
let is_types = conditions == TYPES_CONDITIONS;
if is_types && package_subpath == "." {
if mode.is_types() && package_subpath == "." {
if let Ok(Some(path)) =
legacy_main_resolve(&package_config, referrer_kind, conditions)
legacy_main_resolve(&package_config, referrer_kind, mode)
{
return Ok(Some(path));
} else {
@ -703,7 +697,7 @@ fn package_config_resolve(
}
}
if package_subpath == "." {
return legacy_main_resolve(&package_config, referrer_kind, conditions);
return legacy_main_resolve(&package_config, referrer_kind, mode);
}
Ok(Some(package_dir.join(package_subpath)))
@ -789,12 +783,13 @@ fn module_resolve(
specifier: &str,
referrer: &ModuleSpecifier,
conditions: &[&str],
mode: NodeResolutionMode,
npm_resolver: &dyn RequireNpmResolver,
) -> Result<Option<ModuleSpecifier>, AnyError> {
// note: if we're here, the referrer is an esm module
let url = if should_be_treated_as_relative_or_absolute_path(specifier) {
let resolved_specifier = referrer.join(specifier)?;
if conditions == TYPES_CONDITIONS {
if mode.is_types() {
let file_path = to_file_path(&resolved_specifier);
// todo(dsherret): the node module kind is not correct and we
// should use the value provided by typescript instead
@ -813,6 +808,7 @@ fn module_resolve(
referrer,
NodeModuleKind::Esm,
conditions,
mode,
npm_resolver,
)
.map(|p| ModuleSpecifier::from_file_path(p).unwrap())?,
@ -825,6 +821,7 @@ fn module_resolve(
referrer,
NodeModuleKind::Esm,
conditions,
mode,
npm_resolver,
)?
.map(|p| ModuleSpecifier::from_file_path(p).unwrap())
@ -956,6 +953,7 @@ pub fn translate_cjs_to_esm(
// FIXME(bartlomieju): check if these conditions are okay, probably
// should be `deno-require`, because `deno` is already used in `esm_resolver.rs`
&["deno", "require", "default"],
NodeResolutionMode::Execution,
npm_resolver,
)?;
let reexport_specifier =
@ -1026,6 +1024,7 @@ fn resolve(
specifier: &str,
referrer: &ModuleSpecifier,
conditions: &[&str],
mode: NodeResolutionMode,
npm_resolver: &dyn RequireNpmResolver,
) -> Result<PathBuf, AnyError> {
if specifier.starts_with('/') {
@ -1050,7 +1049,7 @@ fn resolve(
let module_dir = npm_resolver.resolve_package_folder_from_package(
package_specifier.as_str(),
&referrer_path,
conditions,
mode,
)?;
let package_json_path = module_dir.join("package.json");
@ -1066,6 +1065,7 @@ fn resolve(
referrer,
NodeModuleKind::Esm,
conditions,
mode,
npm_resolver,
);
}

View file

@ -10,6 +10,7 @@ use deno_core::error::AnyError;
use deno_core::futures;
use deno_core::futures::future::BoxFuture;
use deno_core::url::Url;
use deno_runtime::deno_node::NodeResolutionMode;
use crate::args::Lockfile;
use crate::npm::cache::should_sync_download;
@ -29,7 +30,7 @@ pub trait InnerNpmPackageResolver: Send + Sync {
&self,
name: &str,
referrer: &ModuleSpecifier,
conditions: &[&str],
mode: NodeResolutionMode,
) -> Result<PathBuf, AnyError>;
fn resolve_package_folder_from_specifier(

View file

@ -12,8 +12,8 @@ use deno_core::error::AnyError;
use deno_core::futures::future::BoxFuture;
use deno_core::futures::FutureExt;
use deno_core::url::Url;
use deno_runtime::deno_node::NodeResolutionMode;
use deno_runtime::deno_node::PackageJson;
use deno_runtime::deno_node::TYPES_CONDITIONS;
use crate::args::Lockfile;
use crate::npm::resolution::NpmResolution;
@ -76,7 +76,7 @@ impl InnerNpmPackageResolver for GlobalNpmPackageResolver {
&self,
name: &str,
referrer: &ModuleSpecifier,
conditions: &[&str],
mode: NodeResolutionMode,
) -> Result<PathBuf, AnyError> {
let referrer_pkg_id = self
.cache
@ -84,7 +84,7 @@ impl InnerNpmPackageResolver for GlobalNpmPackageResolver {
let pkg_result = self
.resolution
.resolve_package_from_package(name, &referrer_pkg_id);
if conditions == TYPES_CONDITIONS && !name.starts_with("@types/") {
if mode.is_types() && !name.starts_with("@types/") {
// When doing types resolution, the package must contain a "types"
// entry, or else it will then search for a @types package
if let Ok(pkg) = pkg_result {

View file

@ -19,8 +19,8 @@ use deno_core::futures::future::BoxFuture;
use deno_core::futures::FutureExt;
use deno_core::url::Url;
use deno_runtime::deno_core::futures;
use deno_runtime::deno_node::NodeResolutionMode;
use deno_runtime::deno_node::PackageJson;
use deno_runtime::deno_node::TYPES_CONDITIONS;
use tokio::task::JoinHandle;
use crate::args::Lockfile;
@ -152,7 +152,7 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver {
&self,
name: &str,
referrer: &ModuleSpecifier,
conditions: &[&str],
mode: NodeResolutionMode,
) -> Result<PathBuf, AnyError> {
let local_path = self.resolve_folder_for_specifier(referrer)?;
let package_root_path = self.resolve_package_root(&local_path);
@ -162,7 +162,7 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver {
let sub_dir = join_package_name(current_folder, name);
if sub_dir.is_dir() {
// if doing types resolution, only resolve the package if it specifies a types property
if conditions == TYPES_CONDITIONS && !name.starts_with("@types/") {
if mode.is_types() && !name.starts_with("@types/") {
let package_json = PackageJson::load_skip_read_permission(
sub_dir.join("package.json"),
)?;
@ -175,7 +175,7 @@ impl InnerNpmPackageResolver for LocalNpmPackageResolver {
}
// if doing type resolution, check for the existance of a @types package
if conditions == TYPES_CONDITIONS && !name.starts_with("@types/") {
if mode.is_types() && !name.starts_with("@types/") {
let sub_dir =
join_package_name(current_folder, &types_package_name(name));
if sub_dir.is_dir() {

View file

@ -11,6 +11,7 @@ use deno_core::error::custom_error;
use deno_core::error::AnyError;
use deno_core::parking_lot::Mutex;
use deno_core::serde_json;
use deno_runtime::deno_node::NodeResolutionMode;
use deno_runtime::deno_node::PathClean;
use deno_runtime::deno_node::RequireNpmResolver;
use global::GlobalNpmPackageResolver;
@ -197,11 +198,11 @@ impl NpmPackageResolver {
&self,
name: &str,
referrer: &ModuleSpecifier,
conditions: &[&str],
mode: NodeResolutionMode,
) -> Result<PathBuf, AnyError> {
let path = self
.inner
.resolve_package_folder_from_package(name, referrer, conditions)?;
.resolve_package_folder_from_package(name, referrer, mode)?;
log::debug!("Resolved {} from {} to {}", name, referrer, path.display());
Ok(path)
}
@ -330,10 +331,10 @@ impl RequireNpmResolver for NpmPackageResolver {
&self,
specifier: &str,
referrer: &std::path::Path,
conditions: &[&str],
mode: NodeResolutionMode,
) -> Result<PathBuf, AnyError> {
let referrer = path_to_specifier(referrer)?;
self.resolve_package_folder_from_package(specifier, &referrer, conditions)
self.resolve_package_folder_from_package(specifier, &referrer, mode)
}
fn resolve_package_folder_from_path(

View file

@ -53,6 +53,7 @@ use deno_graph::source::Resolver;
use deno_graph::ModuleKind;
use deno_graph::Resolved;
use deno_runtime::deno_broadcast_channel::InMemoryBroadcastChannel;
use deno_runtime::deno_node::NodeResolutionMode;
use deno_runtime::deno_tls::rustls::RootCertStore;
use deno_runtime::deno_web::BlobStore;
use deno_runtime::inspector_server::InspectorServer;
@ -514,7 +515,7 @@ impl ProcState {
.handle_node_resolve_result(node::node_resolve(
specifier,
&referrer,
node::NodeResolutionMode::Execution,
NodeResolutionMode::Execution,
&self.npm_resolver,
))
.with_context(|| {
@ -547,7 +548,7 @@ impl ProcState {
return self
.handle_node_resolve_result(node::node_resolve_npm_reference(
&reference,
node::NodeResolutionMode::Execution,
NodeResolutionMode::Execution,
&self.npm_resolver,
))
.with_context(|| format!("Could not resolve '{}'.", reference));

View file

@ -327,6 +327,14 @@ mod npm {
exit_code: 0,
});
itest!(types_exports_import_types {
args: "run --check=all npm/types_exports_import_types/main.ts",
output: "npm/types_exports_import_types/main.out",
envs: env_vars_for_npm_tests(),
http_server: true,
exit_code: 0,
});
itest!(types_no_types_entry {
args: "run --check=all npm/types_no_types_entry/main.ts",
output: "npm/types_no_types_entry/main.out",

View file

@ -0,0 +1 @@
export function getValue(): 5;

View file

@ -0,0 +1,3 @@
export function getValue() {
return 5;
}

View file

@ -0,0 +1,10 @@
{
"name": "@denotest/types-exports-import-types",
"version": "1.0.0",
"exports": {
"node": {
"types": "./dist/main.d.ts",
"import": "./dist/main.mjs"
}
}
}

View file

@ -0,0 +1,4 @@
Download http://localhost:4545/npm/registry/@denotest/types-exports-import-types
Download http://localhost:4545/npm/registry/@denotest/types-exports-import-types/1.0.0.tgz
Check file://[WILDCARD]/types_exports_import_types/main.ts
5

View file

@ -0,0 +1,4 @@
import { getValue } from "npm:@denotest/types-exports-import-types";
const result: 5 = getValue();
console.log(result);

View file

@ -6,7 +6,6 @@ use crate::graph_util::ModuleEntry;
use crate::node;
use crate::node::node_resolve_npm_reference;
use crate::node::NodeResolution;
use crate::node::NodeResolutionMode;
use crate::npm::NpmPackageReference;
use crate::npm::NpmPackageResolver;
use crate::util::checksum;
@ -33,6 +32,7 @@ use deno_core::OpState;
use deno_core::RuntimeOptions;
use deno_core::Snapshot;
use deno_graph::Resolved;
use deno_runtime::deno_node::NodeResolutionMode;
use once_cell::sync::Lazy;
use std::borrow::Cow;
use std::collections::HashMap;
@ -619,7 +619,7 @@ fn op_resolve(
node::node_resolve(
specifier,
&referrer,
node::NodeResolutionMode::Types,
NodeResolutionMode::Types,
npm_resolver,
)
.ok()

View file

@ -30,8 +30,8 @@ pub use resolution::package_imports_resolve;
pub use resolution::package_resolve;
pub use resolution::path_to_declaration_path;
pub use resolution::NodeModuleKind;
pub use resolution::NodeResolutionMode;
pub use resolution::DEFAULT_CONDITIONS;
pub use resolution::TYPES_CONDITIONS;
use std::cell::RefCell;
pub trait NodePermissions {
@ -43,7 +43,7 @@ pub trait RequireNpmResolver {
&self,
specifier: &str,
referrer: &Path,
conditions: &[&str],
mode: NodeResolutionMode,
) -> Result<PathBuf, AnyError>;
fn resolve_package_folder_from_path(
@ -292,7 +292,7 @@ fn op_require_resolve_deno_dir(
.resolve_package_folder_from_package(
&request,
&PathBuf::from(parent_filename),
DEFAULT_CONDITIONS,
NodeResolutionMode::Execution,
)
.ok()
.map(|p| p.to_string_lossy().to_string())
@ -506,6 +506,7 @@ fn op_require_try_self(
&referrer,
NodeModuleKind::Cjs,
resolution::REQUIRE_CONDITIONS,
NodeResolutionMode::Execution,
&*resolver,
)
.map(|r| Some(r.to_string_lossy().to_string()))
@ -568,6 +569,7 @@ fn op_require_resolve_exports(
&referrer,
NodeModuleKind::Cjs,
resolution::REQUIRE_CONDITIONS,
NodeResolutionMode::Execution,
&*resolver,
)
.map(|r| Some(r.to_string_lossy().to_string()))
@ -627,6 +629,7 @@ where
&referrer,
NodeModuleKind::Cjs,
resolution::REQUIRE_CONDITIONS,
NodeResolutionMode::Execution,
&*resolver,
)
.map(|r| Some(Url::from_file_path(r).unwrap().to_string()));

View file

@ -19,7 +19,6 @@ use crate::RequireNpmResolver;
pub static DEFAULT_CONDITIONS: &[&str] = &["deno", "node", "import"];
pub static REQUIRE_CONDITIONS: &[&str] = &["require", "node"];
pub static TYPES_CONDITIONS: &[&str] = &["types"];
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum NodeModuleKind {
@ -27,6 +26,18 @@ pub enum NodeModuleKind {
Cjs,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum NodeResolutionMode {
Execution,
Types,
}
impl NodeResolutionMode {
pub fn is_types(&self) -> bool {
matches!(self, NodeResolutionMode::Types)
}
}
/// Checks if the resolved file has a corresponding declaration file.
pub fn path_to_declaration_path(
path: PathBuf,
@ -175,6 +186,7 @@ pub fn package_imports_resolve(
referrer: &ModuleSpecifier,
referrer_kind: NodeModuleKind,
conditions: &[&str],
mode: NodeResolutionMode,
npm_resolver: &dyn RequireNpmResolver,
) -> Result<PathBuf, AnyError> {
if name == "#" || name.starts_with("#/") || name.ends_with('/') {
@ -202,6 +214,7 @@ pub fn package_imports_resolve(
false,
true,
conditions,
mode,
npm_resolver,
)?;
if let Some(resolved) = maybe_resolved {
@ -243,6 +256,7 @@ pub fn package_imports_resolve(
true,
true,
conditions,
mode,
npm_resolver,
)?;
if let Some(resolved) = maybe_resolved {
@ -306,6 +320,7 @@ fn resolve_package_target_string(
pattern: bool,
internal: bool,
conditions: &[&str],
mode: NodeResolutionMode,
npm_resolver: &dyn RequireNpmResolver,
) -> Result<PathBuf, AnyError> {
if !subpath.is_empty() && !pattern && !target.ends_with('/') {
@ -338,6 +353,7 @@ fn resolve_package_target_string(
&package_json_url,
referrer_kind,
conditions,
mode,
npm_resolver,
) {
Ok(Some(path)) => Ok(path),
@ -412,10 +428,11 @@ fn resolve_package_target(
pattern: bool,
internal: bool,
conditions: &[&str],
mode: NodeResolutionMode,
npm_resolver: &dyn RequireNpmResolver,
) -> Result<Option<PathBuf>, AnyError> {
if let Some(target) = target.as_str() {
return Ok(Some(resolve_package_target_string(
return resolve_package_target_string(
target.to_string(),
subpath,
package_subpath,
@ -425,8 +442,10 @@ fn resolve_package_target(
pattern,
internal,
conditions,
mode,
npm_resolver,
)?));
)
.map(Some);
} else if let Some(target_arr) = target.as_array() {
if target_arr.is_empty() {
return Ok(None);
@ -444,23 +463,25 @@ fn resolve_package_target(
pattern,
internal,
conditions,
mode,
npm_resolver,
);
if let Err(e) = resolved_result {
let err_string = e.to_string();
last_error = Some(e);
if err_string.starts_with("[ERR_INVALID_PACKAGE_TARGET]") {
match resolved_result {
Ok(Some(resolved)) => return Ok(Some(resolved)),
Ok(None) => {
last_error = None;
continue;
}
return Err(last_error.unwrap());
Err(e) => {
let err_string = e.to_string();
last_error = Some(e);
if err_string.starts_with("[ERR_INVALID_PACKAGE_TARGET]") {
continue;
}
return Err(last_error.unwrap());
}
}
let resolved = resolved_result.unwrap();
if resolved.is_none() {
last_error = None;
continue;
}
return Ok(resolved);
}
if last_error.is_none() {
return Ok(None);
@ -475,8 +496,20 @@ fn resolve_package_target(
// Some("\"exports\" cannot contain numeric property keys.".to_string()),
// ));
if key == "default" || conditions.contains(&key.as_str()) {
if key == "default"
|| conditions.contains(&key.as_str())
|| mode.is_types() && key.as_str() == "types"
{
let condition_target = target_obj.get(key).unwrap().to_owned();
if mode.is_types()
&& key.as_str() != "types"
&& condition_target.is_string()
{
// skip because this isn't a types entry
continue;
}
let resolved = resolve_package_target(
package_json_path,
condition_target,
@ -487,12 +520,15 @@ fn resolve_package_target(
pattern,
internal,
conditions,
mode,
npm_resolver,
)?;
if resolved.is_none() {
continue;
match resolved {
Some(resolved) => return Ok(Some(resolved)),
None => {
continue;
}
}
return Ok(resolved);
}
}
} else if target.is_null() {
@ -520,6 +556,7 @@ fn throw_exports_not_found(
)
}
#[allow(clippy::too_many_arguments)]
pub fn package_exports_resolve(
package_json_path: &Path,
package_subpath: String,
@ -527,6 +564,7 @@ pub fn package_exports_resolve(
referrer: &ModuleSpecifier,
referrer_kind: NodeModuleKind,
conditions: &[&str],
mode: NodeResolutionMode,
npm_resolver: &dyn RequireNpmResolver,
) -> Result<PathBuf, AnyError> {
if package_exports.contains_key(&package_subpath)
@ -544,6 +582,7 @@ pub fn package_exports_resolve(
false,
false,
conditions,
mode,
npm_resolver,
)?;
if resolved.is_none() {
@ -602,6 +641,7 @@ pub fn package_exports_resolve(
true,
false,
conditions,
mode,
npm_resolver,
)?;
if let Some(resolved) = maybe_resolved {
@ -678,6 +718,7 @@ pub fn package_resolve(
referrer: &ModuleSpecifier,
referrer_kind: NodeModuleKind,
conditions: &[&str],
mode: NodeResolutionMode,
npm_resolver: &dyn RequireNpmResolver,
) -> Result<Option<PathBuf>, AnyError> {
let (package_name, package_subpath, _is_scoped) =
@ -696,6 +737,7 @@ pub fn package_resolve(
referrer,
referrer_kind,
conditions,
mode,
npm_resolver,
)
.map(Some);
@ -705,7 +747,7 @@ pub fn package_resolve(
let package_dir_path = npm_resolver.resolve_package_folder_from_package(
&package_name,
&referrer.to_file_path().unwrap(),
conditions,
mode,
)?;
let package_json_path = package_dir_path.join("package.json");
@ -732,17 +774,18 @@ pub fn package_resolve(
referrer,
referrer_kind,
conditions,
mode,
npm_resolver,
)
.map(Some);
}
if package_subpath == "." {
return legacy_main_resolve(&package_json, referrer_kind, conditions);
return legacy_main_resolve(&package_json, referrer_kind, mode);
}
let file_path = package_json.path.parent().unwrap().join(&package_subpath);
if conditions == TYPES_CONDITIONS {
if mode.is_types() {
let maybe_declaration_path =
path_to_declaration_path(file_path, referrer_kind);
Ok(maybe_declaration_path)
@ -803,10 +846,9 @@ fn file_exists(path: &Path) -> bool {
pub fn legacy_main_resolve(
package_json: &PackageJson,
referrer_kind: NodeModuleKind,
conditions: &[&str],
mode: NodeResolutionMode,
) -> Result<Option<PathBuf>, AnyError> {
let is_types = conditions == TYPES_CONDITIONS;
let maybe_main = if is_types {
let maybe_main = if mode.is_types() {
match package_json.types.as_ref() {
Some(types) => Some(types),
None => {
@ -832,7 +874,7 @@ pub fn legacy_main_resolve(
}
// todo(dsherret): investigate exactly how node and typescript handles this
let endings = if is_types {
let endings = if mode.is_types() {
match referrer_kind {
NodeModuleKind::Cjs => {
vec![".d.ts", ".d.cts", "/index.d.ts", "/index.d.cts"]
@ -863,7 +905,7 @@ pub fn legacy_main_resolve(
}
}
let index_file_names = if is_types {
let index_file_names = if mode.is_types() {
// todo(dsherret): investigate exactly how typescript does this
match referrer_kind {
NodeModuleKind::Cjs => vec!["index.d.ts", "index.d.cts"],