Skip to content

Don't offer module completions in non-module JS files #27004

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
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
4 changes: 2 additions & 2 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1268,10 +1268,10 @@ namespace ts.Completions {
if (sourceFile.externalModuleIndicator) return true;
// If already using commonjs, don't introduce ES6.
if (sourceFile.commonJsModuleIndicator) return false;
// If some file is using ES6 modules, assume that it's OK to add more.
if (programContainsEs6Modules(program)) return true;
// For JS, stay on the safe side.
if (isUncheckedFile) return false;
// If some file is using ES6 modules, assume that it's OK to add more.
if (programContainsEs6Modules(program)) return true;
// If module transpilation is enabled or we're targeting es6 or above, or not emitting, OK.
return compilerOptionsIndicateEs6Modules(program.getCompilerOptions());
}
Expand Down
13 changes: 8 additions & 5 deletions tests/cases/fourslash/completionsImport_importType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
// @allowJs: true

// @Filename: /a.js
////export const x = 0;
////export class C {}
/////** @typedef {number} T */
//// export const x = 0;
//// export class C {}
//// /** @typedef {number} T */

// @Filename: /b.js
/////** @type {/*0*/} */
/////** @type {/*1*/} */
//// export const m = 0;
//// /** @type {/*0*/} */
//// /** @type {/*1*/} */

verify.completions({
marker: ["0", "1"],
Expand Down Expand Up @@ -43,6 +44,7 @@ verify.applyCodeActionFromCompletion("0", {
newFileContent:
`import { C } from "./a";

export const m = 0;
/** @type {} */
/** @type {} */`,
});
Expand All @@ -55,6 +57,7 @@ verify.applyCodeActionFromCompletion("1", {
newFileContent:
`import { C } from "./a";

export const m = 0;
/** @type {} */
/** @type {import("./a").} */`,
});
31 changes: 31 additions & 0 deletions tests/cases/fourslash/noImportCompletionsInOtherJavaScriptFile.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/// <reference path="fourslash.ts" />

// @allowJs: true
// @module: esnext

// @Filename: /node_modules/foo/index.d.ts
//// export const fail: number;

// @Filename: /a.js
//// export const x = 0;
//// export class C {}
////

// @Filename: /b.js
//// /**/

goTo.file("/b.js");
goTo.marker();
verify.not.completionListContains("fail", undefined, undefined, undefined, undefined, undefined, { includeCompletionsForModuleExports: true });
edit.insert("export const k = 10;\r\nf");
verify.completionListContains(
{ name: "fail", source: "/node_modules/foo/index" },
"const fail: number",
"",
"const",
undefined,
true,
{
includeCompletionsForModuleExports: true,
sourceDisplay: "./node_modules/foo/index"
});