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

fix(node): change resolution mode based on import construct #545

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
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
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
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
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;
Copy link
Member Author

Choose a reason for hiding this comment

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

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
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