module not resolved in JS file using var x = require('...') syntax

[waderyan]

From @timty on October 19, 2016 17:18

Intellisense works great for my postgress Pool type when explicitly declared:

However when passed as a parameter to a function and described through a JSDoc parameter intellisense fails:

• VSCode Version: Code 1.6.1 (9e4e44c19e393803e2b05fe2323cf4ed7e36880e, 2016-10-13T15:27:10.246Z)
• OS Version: Darwin x64 15.6.0
• Extensions: Unity.unity-debug-1.0.1, benjaminromano.typings-installer-0.2.0, dbaeumer.vscode-eslint-1.0.7, dkundel.vscode-npm-source-0.0.3, joelday.docthis-0.3.4, mohsen1.prettify-json-0.0.3, ms-vscode.omnisharp-0.3.3, rafaelmaiolla.diff-0.0.1, stevencl.addDocComments-0.0.8, xabikos.JavaScriptSnippets-1.1.0, xabikos.ReactSnippets-1.1.1

Copied from original issue: microsoft/vscode#14008

[waderyan]

@timty thank you for opening this issue. I have confirmed this with the latest bits and no extensions. Migrating to JavaScript language service.

Code to copy and paste.

class Foo {

    bar() {}
}

var f = new Foo();
// IntelliSense works correctly
// f. 


/**
 * @param {Foo} x
 */
function zoo(x) {
    // IntelliSense is not being given
    x.
}

[sandersn]

@waderyan your code doesn't repro for me on master. Intellisense works fine when I paste the code in a javascript file.

Intellisense does not work for me in a TypeScript file, but Typescript doesn't rely on jsdoc to give types to parameters. It only uses the documentation.

1. When I paste the code into a typescript file that has --noImplicitAny turned on, I get an error at zoo(x).
2. If you change the jsdoc line to read @param {Foo} x documentation, you will see "documentation" in the quick info hover for x, indicating that Intellisense is actually working in typescript too.

@timty Are you using typescript or javascript? In a typescript file you will not get intellisense for the above reason. In a javascript file the code you gave will not work either, because jsdoc requires the parameter name to match:

/**
 * @param {Pool} pool2 <-- Names must match
 */
function myFunction(pool2) {
  pool2./*intellisense should work now*/
}

If you're using typescript, then you have to use the postfix type syntax: function myFunction(pool2: Pool) {

[timty]

@sandersn @waderyan

Thank you both for your responses. I am using javascript (nodejs). Sorry about the typo when recreating the issue. This does not work with correctly matching names either.

var pg = require('pg');

var pool = new pg.Pool("");
//intellisense works...
//pool.

/**
 * @type {Pool}
 */
var pool1;
//no intellisense here...
//pool1.

/**
 * 
 * @param {Pool} pool2
 */
function myFunction(pool2) {
//no intellisense here...
//pool2.

}

Intellisense here:

No Intellisense with JSDoc type:

No Intellisense with JSDoc param:

[alexrock]

The Pool type was not imported in the module, it is visible as pg.Pool

[aluanhaddad]

@alexrock nice catch.

Typescript doesn't rely on jsdoc to give types to parameters. It only uses the documentation.

@sandersn really glad things are staying that way. jsdoc comments are often out of sync with the actual code. Also they are quite ugly. I have a colleague who uses JetBrain's Webstorm and it auto emits all these meaningless jsdoc tags into a pure typescript project... sorry for the rant.

[timty]

@alexrock @aluanhaddad
It was imported via nodes require statement on the top line. Correct? I do get intellisense in this file for the pg.Pool type just not for my var and function parameter.

[aluanhaddad]

@timty you imported the module with the binding pg.
That means that you must qualify all references to exports of the module with pg as in pg.Pool. The issue is that Pool is not in scope.
some options:
A

import pg = require('pg');
/**
 * @type {pg.Pool}
 */
var pool1;

B

import pg = require('pg');
import { Pool } from 'pg';
/**
 * @type {Pool}
 */
var pool1;

[timty]

@aluanhaddad Thanks for the feedback. Got it working with Option B...

var pg = require('pg');
import { Pool } from 'pg';

/**
 * @type {Pool}
 */
var pool1;

However for the sake of completeness this does not seem to work...

var pg = require('pg');
/**
 * @type {pg.Pool}
 */
var pool1;

[sandersn]

I don't think that the language service has special knowledge of the var x = require('pg') pattern, so it doesn't try to resolve 'pg' as a module. @vladima or @andy-ms, can you confirm?

It does understand ES module syntax and the typescript-only import x = require('...') that @aluanhaddad mentioned.

@timty it's good that you can use ES module syntax to work around this. Does Option B need both lines to work?

I'll leave this bug open until we decide whether the language service should also support var x = require('...') in Javascript files.

[sandersn]

@andy-ms poked around the code and there is code that looks for require in Javascript: collectRequireCalls. So it must be failing to work as intended.

[timty]

@sandersn @andy-ms

Your suspicions are correct. No intellisense with the require pattern at least in this case.

No Intellisense here:

var pg = require('pg');

/**
 * @type {Pool}
 */
var pool1;
//no intellisense
pool1.

Intellisense works here:

import { Pool } from 'pg';

/**
 * @type {Pool}
 */
var pool1;
//intellisense works
pool1.

[mhegazy]

The issue here is that there is no Pool in scope. moreover, pg is a value and not a namespace, so there is no type called pg.Pool.

in the case of the ES6 import, there is a type called Pool and then things work.

if we want to make this to work, we need to make sure that pg is a namespace as well as a value, and to use it you will need to fully qualify it as pg.Pool.

[teppeis]

@mhegazy

we need to make sure that pg is a namespace as well as a value, and to use it you will need to fully qualify it as pg.Pool.

Yes! In TS 2.3, checkJs was introduced. So this feature is more important now.

var pg = require('pg');

/** @type {pg.Pool} */
var pool1;

It should work for intellisense and checkJs.

#14377 is another solution for this issue.

[enko]

@vladima you wrote the code for collectRequireCalls. Maybe you can shed some light into this issue? Thanks!

[mhegazy]

I do not think we should treat values as types or namespaces; this will just lead to confusion in the future. #14377 would be the way we would proceed with this request.

[teppeis]

@mhegazy I agree.
Treating values as types requires const type = require('only-for-typing') in JS code.
#14377 is better solution.

[mhegazy]

Seems like another variant of #16489