Skip to content

Commit

Permalink
fix(node): change resolution mode based on import construct (#545)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dsherret
dsherret authored Nov 26, 2024
1 parent 38d9663 commit 0c97977
Show file tree
Hide file tree
Showing 146 changed files with 1,380 additions and 431 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/target
/npm
.vs
.vscode
js/deno_graph_wasm_bg.wasm
js/deno_graph_wasm.generated.d.ts
Expand Down
12 changes: 12 additions & 0 deletions js/test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ Deno.test({
{
"specifier": "./b.js",
"code": {
"resolutionMode": "import",
"specifier": "file:///a/b.js",
"span": {
"start": {
Expand Down Expand Up @@ -270,6 +271,7 @@ Deno.test({
"specifier": "./deno.json",
"code": {
"specifier": "file:///a/deno.json",
"resolutionMode": "import",
"span": {
"start": {
"line": 0,
Expand Down Expand Up @@ -365,6 +367,7 @@ Deno.test({
"specifier": "./b.js",
"code": {
"specifier": "file:///a/b.js",
"resolutionMode": "import",
"span": {
"start": {
"line": 0,
Expand Down Expand Up @@ -461,6 +464,7 @@ Deno.test({
"specifier": "builtin:fs",
"code": {
"specifier": "builtin:fs",
"resolutionMode": "import",
"span": {
"start": {
"line": 0,
Expand All @@ -477,6 +481,7 @@ Deno.test({
"specifier": "https://example.com/bundle",
"code": {
"specifier": "https://example.com/bundle",
"resolutionMode": "import",
"span": {
"start": {
"line": 1,
Expand Down Expand Up @@ -553,6 +558,7 @@ Deno.test({
"dependencies": [{
"specifier": "./a.ts",
"code": {
"resolutionMode": "import",
"specifier": "file:///a/a.ts",
"span": {
"start": { "line": 2, "character": 26 },
Expand All @@ -563,6 +569,7 @@ Deno.test({
"specifier": "./b.ts",
"code": {
"specifier": "file:///a/b.ts",
"resolutionMode": "import",
"span": {
"start": { "line": 3, "character": 27 },
"end": { "line": 3, "character": 35 },
Expand All @@ -572,6 +579,7 @@ Deno.test({
"specifier": "./c.ts",
"code": {
"specifier": "file:///a/c.ts",
"resolutionMode": "import",
"span": {
"start": { "line": 4, "character": 26 },
"end": { "line": 4, "character": 34 },
Expand All @@ -581,6 +589,7 @@ Deno.test({
"specifier": "./d.ts",
"code": {
"specifier": "file:///a/d.ts",
"resolutionMode": "import",
"span": {
"start": { "line": 5, "character": 31 },
"end": { "line": 5, "character": 39 },
Expand Down Expand Up @@ -745,6 +754,7 @@ Deno.test({
"specifier": "./a.json",
"code": {
"specifier": "file:///a/a.json",
"resolutionMode": "import",
"span": {
"start": {
"line": 1,
Expand All @@ -762,6 +772,7 @@ Deno.test({
"specifier": "./b.json",
"code": {
"specifier": "file:///a/b.json",
"resolutionMode": "import",
"span": {
"start": {
"line": 2,
Expand Down Expand Up @@ -916,6 +927,7 @@ Deno.test({
"specifier": "jsr:@denotest/a",
"code": {
"specifier": "jsr:@denotest/a",
"resolutionMode": "import",
"span": {
"start": {
"character": 7,
Expand Down
2 changes: 2 additions & 0 deletions js/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ export interface ResolvedDependency {
* resolvable in the module graph. If there was an error, `error` will be set
* and this will be undefined. */
specifier?: string;
/** Resolution mode used to resolve the dependency. */
resolutionMode?: "import" | "require";
/** Any error encountered when trying to resolved the specifier. If this is
* defined, `specifier` will be undefined. */
error?: string;
Expand Down
10 changes: 5 additions & 5 deletions lib/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use deno_graph::source::LoadFuture;
use deno_graph::source::LoadOptions;
use deno_graph::source::Loader;
use deno_graph::source::NullFileSystem;
use deno_graph::source::ResolutionMode;
use deno_graph::source::ResolutionKind;
use deno_graph::source::ResolveError;
use deno_graph::source::Resolver;
use deno_graph::source::DEFAULT_JSX_IMPORT_SOURCE_MODULE;
Expand Down Expand Up @@ -160,7 +160,7 @@ impl Resolver for JsResolver {
&self,
specifier: &str,
referrer_range: &Range,
_mode: ResolutionMode,
_kind: ResolutionKind,
) -> Result<ModuleSpecifier, ResolveError> {
if let Some(resolve) = &self.maybe_resolve {
let this = JsValue::null();
Expand Down Expand Up @@ -350,7 +350,7 @@ pub async fn js_parse_module(
mod tests {
use super::*;

use deno_graph::Position;
use deno_graph::PositionRange;
use serde_json::from_value;
use serde_json::json;

Expand All @@ -369,8 +369,8 @@ mod tests {
types: ModuleSpecifier::parse("https://deno.land/x/mod.d.ts").unwrap(),
source: Some(Range {
specifier: ModuleSpecifier::parse("file:///package.json").unwrap(),
start: Position::zeroed(),
end: Position::zeroed(),
range: PositionRange::zeroed(),
resolution_mode: None,
})
})
);
Expand Down
143 changes: 128 additions & 15 deletions src/analyzer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@ use serde::Serializer;

use crate::ast::DENO_TYPES_RE;
use crate::graph::Position;
use crate::source::ResolutionMode;
use crate::DefaultModuleAnalyzer;

#[derive(Clone, Copy, Debug, Eq, PartialEq, Deserialize)]
#[derive(Clone, Copy, Debug, Eq, PartialEq, Deserialize, Hash)]
pub struct PositionRange {
#[serde(default = "Position::zeroed")]
pub start: Position,
#[serde(default = "Position::zeroed")]
pub end: Position,
}

Expand All @@ -34,6 +37,11 @@ impl PositionRange {
}
}

/// Determines if a given position is within the range.
pub fn includes(&self, position: Position) -> bool {
(position >= self.start) && (position <= self.end)
}

pub fn from_source_range(
range: SourceRange,
text_info: &SourceTextInfo,
Expand Down Expand Up @@ -197,12 +205,51 @@ pub struct SpecifierWithRange {
pub range: PositionRange,
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub enum TypeScriptTypesResolutionMode {
Require,
Import,
}

impl TypeScriptTypesResolutionMode {
pub fn from_str(text: &str) -> Option<Self> {
match text {
"import" => Some(Self::Import),
"require" => Some(Self::Require),
_ => None,
}
}

pub fn as_deno_graph(&self) -> ResolutionMode {
match self {
Self::Require => ResolutionMode::Require,
Self::Import => ResolutionMode::Import,
}
}
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
#[serde(tag = "type")]
pub enum TypeScriptReference {
Path(SpecifierWithRange),
Types(SpecifierWithRange),
#[serde(rename_all = "camelCase")]
Types {
#[serde(flatten)]
specifier: SpecifierWithRange,
#[serde(skip_serializing_if = "Option::is_none", default)]
resolution_mode: Option<TypeScriptTypesResolutionMode>,
},
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct JsDocImportInfo {
#[serde(flatten)]
pub specifier: SpecifierWithRange,
#[serde(skip_serializing_if = "Option::is_none", default)]
pub resolution_mode: Option<TypeScriptTypesResolutionMode>,
}

/// Information about JS/TS module.
Expand Down Expand Up @@ -231,7 +278,7 @@ pub struct ModuleInfo {
/// Type imports in JSDoc comment blocks (e.g. `{import("./types.d.ts").Type}`)
/// or `@import { SomeType } from "npm:some-module"`.
#[serde(skip_serializing_if = "Vec::is_empty", default)]
pub jsdoc_imports: Vec<SpecifierWithRange>,
pub jsdoc_imports: Vec<JsDocImportInfo>,
}

pub fn module_graph_1_to_2(module_info: &mut serde_json::Value) {
Expand Down Expand Up @@ -481,13 +528,36 @@ mod test {
end: Position::zeroed(),
},
}),
TypeScriptReference::Types(SpecifierWithRange {
text: "b".to_string(),
range: PositionRange {
start: Position::zeroed(),
end: Position::zeroed(),
TypeScriptReference::Types {
specifier: SpecifierWithRange {
text: "b".to_string(),
range: PositionRange {
start: Position::zeroed(),
end: Position::zeroed(),
},
},
}),
resolution_mode: None,
},
TypeScriptReference::Types {
specifier: SpecifierWithRange {
text: "node".to_string(),
range: PositionRange {
start: Position::zeroed(),
end: Position::zeroed(),
},
},
resolution_mode: Some(TypeScriptTypesResolutionMode::Require),
},
TypeScriptReference::Types {
specifier: SpecifierWithRange {
text: "node-esm".to_string(),
range: PositionRange {
start: Position::zeroed(),
end: Position::zeroed(),
},
},
resolution_mode: Some(TypeScriptTypesResolutionMode::Import),
},
]),
self_types_specifier: None,
jsx_import_source: None,
Expand All @@ -507,6 +577,16 @@ mod test {
"type": "types",
"text": "b",
"range": [[0, 0], [0, 0]],
}, {
"type": "types",
"text": "node",
"range": [[0, 0], [0, 0]],
"resolutionMode": "require",
}, {
"type": "types",
"text": "node-esm",
"range": [[0, 0], [0, 0]],
"resolutionMode": "import",
}]
}),
);
Expand Down Expand Up @@ -611,13 +691,38 @@ mod test {
self_types_specifier: None,
jsx_import_source: None,
jsx_import_source_types: None,
jsdoc_imports: Vec::from([SpecifierWithRange {
text: "a".to_string(),
range: PositionRange {
start: Position::zeroed(),
end: Position::zeroed(),
jsdoc_imports: Vec::from([
JsDocImportInfo {
specifier: SpecifierWithRange {
text: "a".to_string(),
range: PositionRange {
start: Position::zeroed(),
end: Position::zeroed(),
},
},
resolution_mode: None,
},
}]),
JsDocImportInfo {
specifier: SpecifierWithRange {
text: "b".to_string(),
range: PositionRange {
start: Position::zeroed(),
end: Position::zeroed(),
},
},
resolution_mode: Some(TypeScriptTypesResolutionMode::Import),
},
JsDocImportInfo {
specifier: SpecifierWithRange {
text: "c".to_string(),
range: PositionRange {
start: Position::zeroed(),
end: Position::zeroed(),
},
},
resolution_mode: Some(TypeScriptTypesResolutionMode::Require),
},
]),
};
run_serialization_test(
&module_info,
Expand All @@ -627,6 +732,14 @@ mod test {
"jsdocImports": [{
"text": "a",
"range": [[0, 0], [0, 0]],
}, {
"text": "b",
"range": [[0, 0], [0, 0]],
"resolutionMode": "import",
}, {
"text": "c",
"range": [[0, 0], [0, 0]],
"resolutionMode": "require",
}]
}),
);
Expand Down
Loading

0 comments on commit 0c97977

Please sign in to comment.