Skip to content

Fix incorrect suggestion for package that bundles types #45507

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

Merged
merged 3 commits into from
Aug 20, 2021
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
33 changes: 23 additions & 10 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,16 +294,20 @@ namespace ts {
}

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

forEachEntry(sf.resolvedModules, r => {
if (r && r.packageId) set.add(r.packageId.name);
if (r && r.packageId) map.set(r.packageId.name, r.extension === Extension.Dts || !!map.get(r.packageId.name));
});
});
return set;
return map;
});

// Cancellation that controls whether or not we can cancel in the middle of type checking.
Expand Down Expand Up @@ -3416,11 +3420,17 @@ namespace ts {
/*details*/ undefined,
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,
packageId.name, mangleScopedPackageName(packageId.name))
: chainDiagnosticMessages(
/*details*/ undefined,
Diagnostics.Try_npm_i_save_dev_types_Slash_1_if_it_exists_or_add_a_new_declaration_d_ts_file_containing_declare_module_0,
moduleReference,
mangleScopedPackageName(packageId.name))
: packageBundlesTypes(packageId.name)
? chainDiagnosticMessages(
/*details*/ undefined,
Diagnostics.If_the_0_package_actually_exposes_this_module_try_adding_a_new_declaration_d_ts_file_containing_declare_module_1,
packageId.name,
moduleReference)
: chainDiagnosticMessages(
/*details*/ undefined,
Diagnostics.Try_npm_i_save_dev_types_Slash_1_if_it_exists_or_add_a_new_declaration_d_ts_file_containing_declare_module_0,
moduleReference,
mangleScopedPackageName(packageId.name))
: undefined;
errorOrSuggestion(isError, errorNode, chainDiagnosticMessages(
errorInfo,
Expand All @@ -3429,7 +3439,10 @@ namespace ts {
resolvedFileName));
}
function typesPackageExists(packageName: string): boolean {
return getPackagesSet().has(getTypesPackageName(packageName));
return getPackagesMap().has(getTypesPackageName(packageName));
}
function packageBundlesTypes(packageName: string): boolean {
return !!getPackagesMap().get(packageName);
}

function resolveExternalModuleSymbol(moduleSymbol: Symbol, dontResolveAlias?: boolean): Symbol;
Expand Down
5 changes: 5 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -6027,6 +6027,11 @@
"category": "Error",
"code": 7057
},
"If the '{0}' package actually exposes this module, try adding a new declaration (.d.ts) file containing `declare module '{1}';`": {
"category": "Error",
"code": 7058
},


"You cannot rename this element.": {
"category": "Error",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
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.
If the 'foo' package actually exposes this module, try adding a new declaration (.d.ts) file containing `declare module 'foo/other';`


==== tests/cases/conformance/moduleResolution/index.ts (1 errors) ====
import * as Foo from "foo";
import * as Other from "foo/other"/*1*/;
~~~~~~~~~~~
!!! 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.
!!! error TS7016: If the 'foo' package actually exposes this module, try adding a new declaration (.d.ts) file containing `declare module 'foo/other';`
==== tests/cases/conformance/moduleResolution/node_modules/foo/package.json (0 errors) ====
{
"name": "foo",
"version": "1.0.0"
}

==== tests/cases/conformance/moduleResolution/node_modules/foo/index.js (0 errors) ====
var foo = 0;
module.exports = foo;

==== tests/cases/conformance/moduleResolution/node_modules/foo/index.d.ts (0 errors) ====
declare const foo: any;
export = foo;

==== tests/cases/conformance/moduleResolution/node_modules/foo/other.js (0 errors) ====
module.exports = {};

Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
//// [tests/cases/conformance/moduleResolution/declarationNotFoundPackageBundlesTypes.ts] ////

//// [package.json]
{
"name": "foo",
"version": "1.0.0"
}

//// [index.js]
var foo = 0;
module.exports = foo;

//// [index.d.ts]
declare const foo: any;
export = foo;

//// [other.js]
module.exports = {};

//// [index.ts]
import * as Foo from "foo";
import * as Other from "foo/other"/*1*/;

//// [index.js]
"use strict";
exports.__esModule = true;
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
=== tests/cases/conformance/moduleResolution/index.ts ===
import * as Foo from "foo";
>Foo : Symbol(Foo, Decl(index.ts, 0, 6))

import * as Other from "foo/other"/*1*/;
>Other : Symbol(Other, Decl(index.ts, 1, 6))

=== tests/cases/conformance/moduleResolution/node_modules/foo/index.d.ts ===
declare const foo: any;
>foo : Symbol(foo, Decl(index.d.ts, 0, 13))

export = foo;
>foo : Symbol(foo, Decl(index.d.ts, 0, 13))

Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
=== tests/cases/conformance/moduleResolution/index.ts ===
import * as Foo from "foo";
>Foo : any

import * as Other from "foo/other"/*1*/;
>Other : any

=== tests/cases/conformance/moduleResolution/node_modules/foo/index.d.ts ===
declare const foo: any;
>foo : any

export = foo;
>foo : any

Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// @noImplicitAny: true
// @noImplicitReferences: true

// @filename: node_modules/foo/package.json
{
"name": "foo",
"version": "1.0.0"
}

// @filename: node_modules/foo/index.js
var foo = 0;
module.exports = foo;

// @filename: node_modules/foo/index.d.ts
declare const foo: any;
export = foo;

// @filename: node_modules/foo/other.js
module.exports = {};

// @filename: index.ts
import * as Foo from "foo";
import * as Other from "foo/other"/*1*/;