Don't offer import completions in non-module files unless "--module" is set#22951
Don't offer import completions in non-module files unless "--module" is set#229513 commits merged into
Conversation
d08ad02 to
21f4a16
Compare
| } | ||
|
|
||
| // Don't suggest import completions for a commonjs-only module | ||
| if (preferences.includeCompletionsForModuleExports && !(sourceFile.commonJsModuleIndicator && !sourceFile.externalModuleIndicator)) { |
There was a problem hiding this comment.
i am a bit confused now. why was not that working already? in the issue the file is not a module (i.e. no externalModuleIndicator and no commonJsModuleIndicator), so we should not have included the completions, right?
There was a problem hiding this comment.
also should not this just be if (preferences.includeCompletionsForModuleExports && sourceFile.externalModuleIndicator) ?
There was a problem hiding this comment.
I was thinking that even in a currently import-less file we should still add import completions if the user has --module set in their tsconfig. (see #22750 (comment))
There was a problem hiding this comment.
The reason it wasn't working already is we would always add import completions for a file with no commonjs imports/exports.
There was a problem hiding this comment.
I thought that is what #22880 was doing.. but looking at it again, the condition does not seem right.. the problem seems to be more complicated than i originally thought..
how about somethign like:
function shouldOfferImportCompletions(): boolean {
if (preferences.includeCompletionsForModuleExports) {
// sure to have a module
if (sourceFile.externalModuleIndicator) return true;
// sure to have a .js file with a require, do not mess with that file
if (sourceFile.commonJsModuleIndicator) return false;
// either this file is not really a module, or it is a new file added to the project
// see if there are other "source" files that we can infer from
if (find(program.getSourceFiles(), s => !s.isDeclarationFile && !program.isSourceFileFromExternalLibrary(s) && !!s.externalModuleIndicator))
return true;
// either this is an empty project, or one with no modules in it
// for .js files we should fall on the safe side -- i think :)
if (isInJavaScriptFile(sourceFile)) return false;
// in the absense of any other info, then may be we can find something in the compiler options
if (compilerOptions.module !== undefined) return compilerOptions.module > ModuleKind.None;
if (compilerOptions.target >= ScriptTarget.ES2016) return true;
if (compilerOptions.noEmit) return true;
}
return false;
}|
@DanielRosenwasser Open a new issue if you want us to do something about UMD globals here. |
Fixes #22750
Note: does not include the logic for looking through a source file for a UMD use. Personally, I think we should just fixx #10178 (intentional misspelling to prevent GH from closing that) so that using modules doesn't break your existing code!