Skip to content

Commit 84b0578

Browse files
gabrittogabritto
authored
Fix incorrect suggestion for package that bundles types (microsoft#45507)
* Fix incorrect suggestion for package that bundles types * determine if a package ships types from its files * update new error message
1 parent e00722f commit 84b0578

File tree

7 files changed

+132
-10
lines changed

7 files changed

+132
-10
lines changed

‎src/compiler/checker.ts‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -294,16 +294,20 @@ namespace ts {
294294
}
295295

296296
export function createTypeChecker(host: TypeCheckerHost, produceDiagnostics: boolean): TypeChecker {
297-
const getPackagesSet = memoize(() => {
298-
const set = new Set<string>();
297+
const getPackagesMap = memoize(() => {
298+
// A package name maps to true when we detect it has .d.ts files.
299+
// This is useful as an approximation of whether a package bundles its own types.
300+
// Note: we only look at files already found by module resolution,
301+
// so there may be files we did not consider.
302+
const map = new Map<string, boolean>();
299303
host.getSourceFiles().forEach(sf => {
300304
if (!sf.resolvedModules) return;
301305

302306
forEachEntry(sf.resolvedModules, r => {
303-
if (r && r.packageId) set.add(r.packageId.name);
307+
if (r && r.packageId) map.set(r.packageId.name, r.extension === Extension.Dts || !!map.get(r.packageId.name));
304308
});
305309
});
306-
return set;
310+
return map;
307311
});
308312

309313
// Cancellation that controls whether or not we can cancel in the middle of type checking.
@@ -3417,11 +3421,17 @@ namespace ts {
34173421
/*details*/ undefined,
34183422
Diagnostics.If_the_0_package_actually_exposes_this_module_consider_sending_a_pull_request_to_amend_https_Colon_Slash_Slashgithub_com_SlashDefinitelyTyped_SlashDefinitelyTyped_Slashtree_Slashmaster_Slashtypes_Slash_1,
34193423
packageId.name, mangleScopedPackageName(packageId.name))
3420-
: chainDiagnosticMessages(
3421-
/*details*/ undefined,
3422-
Diagnostics.Try_npm_i_save_dev_types_Slash_1_if_it_exists_or_add_a_new_declaration_d_ts_file_containing_declare_module_0,
3423-
moduleReference,
3424-
mangleScopedPackageName(packageId.name))
3424+
: packageBundlesTypes(packageId.name)
3425+
? chainDiagnosticMessages(
3426+
/*details*/ undefined,
3427+
Diagnostics.If_the_0_package_actually_exposes_this_module_try_adding_a_new_declaration_d_ts_file_containing_declare_module_1,
3428+
packageId.name,
3429+
moduleReference)
3430+
: chainDiagnosticMessages(
3431+
/*details*/ undefined,
3432+
Diagnostics.Try_npm_i_save_dev_types_Slash_1_if_it_exists_or_add_a_new_declaration_d_ts_file_containing_declare_module_0,
3433+
moduleReference,
3434+
mangleScopedPackageName(packageId.name))
34253435
: undefined;
34263436
errorOrSuggestion(isError, errorNode, chainDiagnosticMessages(
34273437
errorInfo,
@@ -3430,7 +3440,10 @@ namespace ts {
34303440
resolvedFileName));
34313441
}
34323442
function typesPackageExists(packageName: string): boolean {
3433-
return getPackagesSet().has(getTypesPackageName(packageName));
3443+
return getPackagesMap().has(getTypesPackageName(packageName));
3444+
}
3445+
function packageBundlesTypes(packageName: string): boolean {
3446+
return !!getPackagesMap().get(packageName);
34343447
}
34353448

34363449
function resolveExternalModuleSymbol(moduleSymbol: Symbol, dontResolveAlias?: boolean): Symbol;

‎src/compiler/diagnosticMessages.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6027,6 +6027,11 @@
60276027
"category": "Error",
60286028
"code": 7057
60296029
},
6030+
"If the '{0}' package actually exposes this module, try adding a new declaration (.d.ts) file containing `declare module '{1}';`": {
6031+
"category": "Error",
6032+
"code": 7058
6033+
},
6034+
60306035

60316036
"You cannot rename this element.": {
60326037
"category": "Error",

‎tests/baselines/reference/declarationNotFoundPackageBundlesTypes.errors.txt‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
tests/cases/conformance/moduleResolution/index.ts(2,24): error TS7016: Could not find a declaration file for module 'foo/other'. 'tests/cases/conformance/moduleResolution/node_modules/foo/other.js' implicitly has an 'any' type.
2+
If the 'foo' package actually exposes this module, try adding a new declaration (.d.ts) file containing `declare module 'foo/other';`
3+
4+
5+
==== tests/cases/conformance/moduleResolution/index.ts (1 errors) ====
6+
import * as Foo from "foo";
7+
import * as Other from "foo/other"/*1*/;
8+
~~~~~~~~~~~
9+
!!! error TS7016: Could not find a declaration file for module 'foo/other'. 'tests/cases/conformance/moduleResolution/node_modules/foo/other.js' implicitly has an 'any' type.
10+
!!! error TS7016: If the 'foo' package actually exposes this module, try adding a new declaration (.d.ts) file containing `declare module 'foo/other';`
11+
==== tests/cases/conformance/moduleResolution/node_modules/foo/package.json (0 errors) ====
12+
{
13+
"name": "foo",
14+
"version": "1.0.0"
15+
}
16+
17+
==== tests/cases/conformance/moduleResolution/node_modules/foo/index.js (0 errors) ====
18+
var foo = 0;
19+
module.exports = foo;
20+
21+
==== tests/cases/conformance/moduleResolution/node_modules/foo/index.d.ts (0 errors) ====
22+
declare const foo: any;
23+
export = foo;
24+
25+
==== tests/cases/conformance/moduleResolution/node_modules/foo/other.js (0 errors) ====
26+
module.exports = {};
27+

‎tests/baselines/reference/declarationNotFoundPackageBundlesTypes.js‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
//// [tests/cases/conformance/moduleResolution/declarationNotFoundPackageBundlesTypes.ts] ////
2+
3+
//// [package.json]
4+
{
5+
"name": "foo",
6+
"version": "1.0.0"
7+
}
8+
9+
//// [index.js]
10+
var foo = 0;
11+
module.exports = foo;
12+
13+
//// [index.d.ts]
14+
declare const foo: any;
15+
export = foo;
16+
17+
//// [other.js]
18+
module.exports = {};
19+
20+
//// [index.ts]
21+
import * as Foo from "foo";
22+
import * as Other from "foo/other"/*1*/;
23+
24+
//// [index.js]
25+
"use strict";
26+
exports.__esModule = true;

‎tests/baselines/reference/declarationNotFoundPackageBundlesTypes.symbols‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
=== tests/cases/conformance/moduleResolution/index.ts ===
2+
import * as Foo from "foo";
3+
>Foo : Symbol(Foo, Decl(index.ts, 0, 6))
4+
5+
import * as Other from "foo/other"/*1*/;
6+
>Other : Symbol(Other, Decl(index.ts, 1, 6))
7+
8+
=== tests/cases/conformance/moduleResolution/node_modules/foo/index.d.ts ===
9+
declare const foo: any;
10+
>foo : Symbol(foo, Decl(index.d.ts, 0, 13))
11+
12+
export = foo;
13+
>foo : Symbol(foo, Decl(index.d.ts, 0, 13))
14+

‎tests/baselines/reference/declarationNotFoundPackageBundlesTypes.types‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
=== tests/cases/conformance/moduleResolution/index.ts ===
2+
import * as Foo from "foo";
3+
>Foo : any
4+
5+
import * as Other from "foo/other"/*1*/;
6+
>Other : any
7+
8+
=== tests/cases/conformance/moduleResolution/node_modules/foo/index.d.ts ===
9+
declare const foo: any;
10+
>foo : any
11+
12+
export = foo;
13+
>foo : any
14+

‎tests/cases/conformance/moduleResolution/declarationNotFoundPackageBundlesTypes.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// @noImplicitAny: true
2+
// @noImplicitReferences: true
3+
4+
// @filename: node_modules/foo/package.json
5+
{
6+
"name": "foo",
7+
"version": "1.0.0"
8+
}
9+
10+
// @filename: node_modules/foo/index.js
11+
var foo = 0;
12+
module.exports = foo;
13+
14+
// @filename: node_modules/foo/index.d.ts
15+
declare const foo: any;
16+
export = foo;
17+
18+
// @filename: node_modules/foo/other.js
19+
module.exports = {};
20+
21+
// @filename: index.ts
22+
import * as Foo from "foo";
23+
import * as Other from "foo/other"/*1*/;

0 commit comments

Comments
 (0)