Skip to content

Commit 6aaed85

Browse files
authored
fix(check): improve node types handling (denoland#31677)
1. Always injects node types when someone has a `node:` import (like before, so this isn't dependent on the lib setting... which fixes the linked issue) 2. Handles someone doing `/// <reference lib="node" />`. Previously this was slightly broken. * denoland/TypeScript@c5cb3b2 * denoland/typescript-go#18 Closes denoland#31649
1 parent d366855 commit 6aaed85

File tree

15 files changed

+206
-21
lines changed

15 files changed

+206
-21
lines changed

cli/lsp/documents.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
use std::borrow::Cow;
44
use std::collections::BTreeMap;
55
use std::collections::HashMap;
6+
use std::collections::HashSet;
67
use std::fs;
78
use std::future::Future;
89
use std::ops::Range;
@@ -1506,6 +1507,11 @@ impl DocumentModules {
15061507
for dependency in module.dependencies.values() {
15071508
let code_specifier = dependency.get_code();
15081509
let type_specifier = dependency.get_type();
1510+
if let Some(dep) = code_specifier
1511+
&& dep.scheme() == "node"
1512+
{
1513+
dep_info.has_node_specifier = true;
1514+
}
15091515
if dependency.maybe_deno_types_specifier.is_some()
15101516
&& let (Some(code_specifier), Some(type_specifier)) =
15111517
(code_specifier, type_specifier)
@@ -1587,6 +1593,15 @@ impl DocumentModules {
15871593
.clone()
15881594
}
15891595

1596+
pub fn scopes_with_node_specifier(&self) -> HashSet<Option<Arc<Url>>> {
1597+
self
1598+
.dep_info_by_scope()
1599+
.iter()
1600+
.filter(|(_, i)| i.has_node_specifier)
1601+
.map(|(s, _)| s.clone())
1602+
.collect::<HashSet<_>>()
1603+
}
1604+
15901605
#[cfg_attr(feature = "lsp-tracing", tracing::instrument(skip_all))]
15911606
pub fn resolve(
15921607
&self,

cli/lsp/resolver.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,7 @@ impl LspResolver {
602602
#[derive(Debug, Default, Clone)]
603603
pub struct ScopeDepInfo {
604604
pub deno_types_to_code_resolutions: HashMap<ModuleSpecifier, ModuleSpecifier>,
605+
pub has_node_specifier: bool,
605606
}
606607

607608
#[derive(Debug, Clone, Eq, PartialEq, Hash)]

cli/lsp/tsc.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5094,6 +5094,11 @@ fn op_script_names(state: &mut OpState) -> ScriptNames {
50945094
by_notebook_uri: Default::default(),
50955095
};
50965096

5097+
let scopes_with_node_specifier = state
5098+
.state_snapshot
5099+
.document_modules
5100+
.scopes_with_node_specifier();
5101+
50975102
// Insert global scripts.
50985103
for (compiler_options_key, compiler_options_data) in
50995104
state.state_snapshot.compiler_options_resolver.entries()
@@ -5111,6 +5116,9 @@ fn op_script_names(state: &mut OpState) -> ScriptNames {
51115116
.state_snapshot
51125117
.resolver
51135118
.get_scoped_resolver(scope.as_deref());
5119+
if scopes_with_node_specifier.contains(&scope) {
5120+
script_names.insert("asset:///reference_types_node.d.ts".to_string());
5121+
}
51145122
for (referrer, relative_specifiers) in compiler_options_data
51155123
.ts_config_files
51165124
.iter()

cli/tsc/00_typescript.js

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -127196,7 +127196,7 @@ function createCreateProgramOptions(rootNames, options, host, oldProgram, config
127196127196
};
127197127197
}
127198127198
function createProgram(_rootNamesOrOptions, _options, _host, _oldProgram, _configFileParsingDiagnostics) {
127199-
var _a, _b, _c, _d, _e, _f, _g, _h, _i, _j, _k, _l, _m, _n, _o, _p;
127199+
var _a, _b, _c, _d, _e, _f, _g, _h, _i, _j, _k, _l, _m, _n, _o, _p, _q;
127200127200
let _createProgramOptions = isArray(_rootNamesOrOptions) ? createCreateProgramOptions(_rootNamesOrOptions, _options, _host, _oldProgram, _configFileParsingDiagnostics) : _rootNamesOrOptions;
127201127201
const { rootNames, options, configFileParsingDiagnostics, projectReferences, typeScriptVersion: typeScriptVersion2, host: createProgramOptionsHost } = _createProgramOptions;
127202127202
let { oldProgram } = _createProgramOptions;
@@ -127331,6 +127331,8 @@ function createProgram(_rootNamesOrOptions, _options, _host, _oldProgram, _confi
127331127331
let redirectTargetsMap = createMultiMap();
127332127332
let usesUriStyleNodeCoreModules;
127333127333
const filesByName = /* @__PURE__ */ new Map();
127334+
let shouldLoadNodeTypes = false;
127335+
let foundNodeTypes = false;
127334127336
let missingFileNames = /* @__PURE__ */ new Map();
127335127337
const filesByNameIgnoreCase = host.useCaseSensitiveFileNames() ? /* @__PURE__ */ new Map() : void 0;
127336127338
let resolvedProjectReferences;
@@ -127438,13 +127440,6 @@ function createProgram(_rootNamesOrOptions, _options, _host, _oldProgram, _confi
127438127440
);
127439127441
} else {
127440127442
forEach(options.lib, (libFileName, index) => {
127441-
if (libFileName === "lib.node.d.ts") {
127442-
for (const path of filesByName.keys()) {
127443-
if (deno_exports.isTypesNodePkgPath(path)) {
127444-
return;
127445-
}
127446-
}
127447-
}
127448127443
processRootFile(
127449127444
pathForLibFile(libFileName),
127450127445
/*isDefaultLib*/
@@ -127456,6 +127451,25 @@ function createProgram(_rootNamesOrOptions, _options, _host, _oldProgram, _confi
127456127451
});
127457127452
}
127458127453
}
127454+
const hasTypesNodePackage = () => {
127455+
for (const path of filesByName.keys()) {
127456+
if (deno_exports.isTypesNodePkgPath(path)) {
127457+
return true;
127458+
}
127459+
}
127460+
return false;
127461+
};
127462+
if (foundNodeTypes && !hasTypesNodePackage()) {
127463+
shouldLoadNodeTypes = true;
127464+
processRootFile(
127465+
"asset:///lib.node.d.ts",
127466+
/*isDefaultLib*/
127467+
true,
127468+
/*ignoreNoDefaultLib*/
127469+
false,
127470+
{ kind: 6 /* LibFile */, index: ((_o = options.lib) == null ? void 0 : _o.length) ?? 0 }
127471+
);
127472+
}
127459127473
files = toSorted(processingDefaultLibFiles, compareDefaultLibFiles).concat(processingOtherFiles);
127460127474
processingDefaultLibFiles = void 0;
127461127475
processingOtherFiles = void 0;
@@ -127576,7 +127590,7 @@ function createProgram(_rootNamesOrOptions, _options, _host, _oldProgram, _confi
127576127590
readFile,
127577127591
directoryExists,
127578127592
getSymlinkCache,
127579-
realpath: (_o = host.realpath) == null ? void 0 : _o.bind(host),
127593+
realpath: (_p = host.realpath) == null ? void 0 : _p.bind(host),
127580127594
useCaseSensitiveFileNames: () => host.useCaseSensitiveFileNames(),
127581127595
getCanonicalFileName,
127582127596
getFileIncludeReasons: () => programDiagnostics.getFileReasons(),
@@ -127590,7 +127604,7 @@ function createProgram(_rootNamesOrOptions, _options, _host, _oldProgram, _confi
127590127604
}
127591127605
mark("afterProgram");
127592127606
measure("Program", "beforeProgram", "afterProgram");
127593-
(_p = tracing) == null ? void 0 : _p.pop();
127607+
(_q = tracing) == null ? void 0 : _q.pop();
127594127608
return program;
127595127609
function getResolvedModule(file, moduleName, mode) {
127596127610
var _a2;
@@ -128812,6 +128826,10 @@ function createProgram(_rootNamesOrOptions, _options, _host, _oldProgram, _confi
128812128826
}
128813128827
}
128814128828
function processSourceFile(fileName, isDefaultLib, ignoreNoDefaultLib, packageId, reason) {
128829+
if (fileName === "asset:///lib.node.d.ts" && !shouldLoadNodeTypes) {
128830+
foundNodeTypes = true;
128831+
return;
128832+
}
128815128833
getSourceFileFromReferenceWorker(
128816128834
fileName,
128817128835
(fileName2) => findSourceFile(fileName2, isDefaultLib, ignoreNoDefaultLib, reason, packageId),

cli/tsc/go/tsgo_version.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,15 @@ impl Hashes {
2323
}
2424
}
2525

26-
pub const VERSION: &str = "0.1.12";
26+
pub const VERSION: &str = "0.1.13";
2727
pub const DOWNLOAD_BASE_URL: &str =
28-
"https://github.com/denoland/typescript-go/releases/download/v0.1.12";
28+
"https://github.com/denoland/typescript-go/releases/download/v0.1.13";
2929
pub const HASHES: Hashes = Hashes {
30-
windows_x64: "sha256:6ee2dce2752cbf67ee118fb6568bf0436aef6676cdf59afb4cc6075d2ad58994",
31-
macos_x64: "sha256:deecdf9f080347865df6b48031fb893e0cf0d87e8fa3e73c922d7e420cdd5c18",
32-
macos_arm64: "sha256:e9cd03c44ae4c4be56bfaea53191851b7aed1059ffe6e2dadb38e6171483b20c",
33-
linux_x64: "sha256:f7640e41fff1830fdb2a7d0aefbe8a5941e31de5a547eda43ec60d5b75b1605d",
34-
linux_arm64: "sha256:abf296d2bfd0fb3a4eceb3e55ea5dc9d1ed6683ada9cf4f36370e2f1eecde36e",
30+
windows_x64: "sha256:0d884056a42af3943a2d9b17eb862f199a5b763d132506036205853ffd499142",
31+
macos_x64: "sha256:5d34443bd0af95debbc9edbfea1ae6d45d78af26a6da1fe5f547b5824b63c2d0",
32+
macos_arm64: "sha256:e93c671629e6929cb0b3ebad2a8834641b77e4953c3038a50b29daeb2f36e070",
33+
linux_x64: "sha256:dd29cd82d8f89bc70d23011e9cc687d7d4d9631ef395ffeac43f605ab8409255",
34+
linux_arm64: "sha256:bf6380027b63e3758cdd29be7d35daaa02371ce60e285b7da3d37a2c3f950fc0",
3535
};
3636

3737
const _: () = {

cli/tsc/mod.rs

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,19 @@ pub static LAZILY_LOADED_STATIC_ASSETS: Lazy<
312312
maybe_compressed_lib!("lib.webworker.d.ts"),
313313
maybe_compressed_lib!("lib.webworker.importscripts.d.ts"),
314314
maybe_compressed_lib!("lib.webworker.iterable.d.ts"),
315+
(
316+
// Special file that can be used to inject the @types/node package.
317+
// This is used for `node:` specifiers.
318+
"reference_types_node.d.ts",
319+
StaticAsset {
320+
is_lib: false,
321+
source: StaticAssetSource::Uncompressed(
322+
// causes either the built-in node types to be used or it
323+
// prefers the @types/node if it exists
324+
"/// <reference lib=\"node\" />\n/// <reference types=\"npm:@types/node\" />\n",
325+
),
326+
},
327+
),
315328
])
316329
.into_iter()
317330
.chain(node_type_libs!())
@@ -767,7 +780,36 @@ fn resolve_non_graph_specifier_types(
767780
.ok(),
768781
)))
769782
} else {
770-
Ok(None)
783+
match NpmPackageReqReference::from_str(raw_specifier) {
784+
Ok(npm_req_ref) => {
785+
debug_assert_eq!(resolution_mode, ResolutionMode::Import);
786+
// This could occur when resolving npm:@types/node when it is
787+
// injected and not part of the graph
788+
let package_folder =
789+
npm.npm_resolver.resolve_pkg_folder_from_deno_module_req(
790+
npm_req_ref.req(),
791+
referrer,
792+
)?;
793+
let res_result = node_resolver
794+
.resolve_package_subpath_from_deno_module(
795+
&package_folder,
796+
npm_req_ref.sub_path(),
797+
Some(referrer),
798+
resolution_mode,
799+
NodeResolutionKind::Types,
800+
);
801+
let maybe_url = match res_result {
802+
Ok(url_or_path) => Some(url_or_path.into_url()?),
803+
Err(err) => match err.code() {
804+
NodeJsErrorCode::ERR_MODULE_NOT_FOUND
805+
| NodeJsErrorCode::ERR_TYPES_NOT_FOUND => None,
806+
_ => return Err(err.into()),
807+
},
808+
};
809+
Ok(Some(into_specifier_and_media_type(maybe_url)))
810+
}
811+
_ => Ok(None),
812+
}
771813
}
772814
}
773815

@@ -943,7 +985,10 @@ pub fn resolve_specifier_for_tsc(
943985
) => {
944986
// it's most likely requesting the jsxImportSource, which isn't loaded
945987
// into the graph when not using jsx, so just ignore this error
946-
if specifier.ends_with("/jsx-runtime") {
988+
if specifier.ends_with("/jsx-runtime")
989+
// ignore in order to support attempt to load when it doesn't exist
990+
|| specifier == "npm:@types/node"
991+
{
947992
None
948993
} else {
949994
return Err(err.into());

cli/type_checker.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,7 @@ struct GraphWalker<'a> {
650650
maybe_hasher: Option<FastInsecureHasher>,
651651
seen: HashSet<&'a Url>,
652652
pending: VecDeque<(&'a Url, bool)>,
653+
has_seen_node_builtin: bool,
653654
roots: Vec<(ModuleSpecifier, MediaType)>,
654655
missing_diagnostics: tsc::Diagnostics,
655656
}
@@ -689,6 +690,7 @@ impl<'a> GraphWalker<'a> {
689690
graph.imports.len() + graph.specifiers_count(),
690691
),
691692
pending: VecDeque::new(),
693+
has_seen_node_builtin: false,
692694
roots: Vec::with_capacity(graph.imports.len() + graph.specifiers_count()),
693695
missing_diagnostics: Default::default(),
694696
}
@@ -736,7 +738,14 @@ impl<'a> GraphWalker<'a> {
736738
/// redirects resolved. We need to include all the emittable files in
737739
/// the roots, so they get type checked and optionally emitted,
738740
/// otherwise they would be ignored if only imported into JavaScript.
739-
pub fn into_tsc_roots(self) -> TscRoots {
741+
pub fn into_tsc_roots(mut self) -> TscRoots {
742+
if self.has_seen_node_builtin && !self.roots.is_empty() {
743+
// inject a specifier that will force node types to be resolved
744+
self.roots.push((
745+
ModuleSpecifier::parse("asset:///reference_types_node.d.ts").unwrap(),
746+
MediaType::Dts,
747+
));
748+
}
740749
TscRoots {
741750
roots: self.roots,
742751
missing_diagnostics: self.missing_diagnostics,
@@ -788,14 +797,19 @@ impl<'a> GraphWalker<'a> {
788797
Module::Wasm(module) => {
789798
maybe_module_dependencies = Some(&module.dependencies);
790799
}
791-
Module::Json(_) | Module::Node(_) | Module::Npm(_) => {}
800+
Module::Json(_) | Module::Npm(_) => {}
792801
Module::External(module) => {
793802
// NPM files for `"nodeModulesDir": "manual"`.
794803
let media_type = MediaType::from_specifier(&module.specifier);
795804
if media_type.is_declaration() {
796805
self.roots.push((module.specifier.clone(), media_type));
797806
}
798807
}
808+
Module::Node(_) => {
809+
if !self.has_seen_node_builtin {
810+
self.has_seen_node_builtin = true;
811+
}
812+
}
799813
}
800814

801815
if module.media_type().is_declaration() {
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"args": "check -q --all main.ts",
3+
"output": "",
4+
"variants": {
5+
"tsgo": {
6+
"use_tsgo": "1"
7+
},
8+
"tsc": {
9+
"use_tsgo": ""
10+
}
11+
},
12+
"envs": {
13+
"DENO_UNSTABLE_TSGO": "${use_tsgo}"
14+
}
15+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"compilerOptions": {
3+
// Notice there's no "node" here. In this case, it
4+
// should inject and use the built-in types
5+
"lib": ["deno.window"]
6+
}
7+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { test } from "node:test";
2+
3+
test("test", (ctx) => {
4+
console.log(ctx);
5+
});

0 commit comments

Comments
 (0)