Unhelpful "Did you forget to include 'void' in your type argument to 'Promise'" in JS file

[JacksonKearl]

TS Template added by @mjbvz

TypeScript Version: 4.5.0-dev.20211028

Search Terms

• javascript
• checkjs
• strict

See #46570 (comment) for more minimal repro

---

In minimal-hello-world extension sample, using this code:

// The module 'vscode' contains the VS Code extensibility API
// Import the module and reference it with the alias vscode in your code below
const vscode = require('vscode');

// this method is called when your extension is activated
// your extension is activated the very first time the command is executed

/**
 * @param {vscode.ExtensionContext} context
 */
function activate(context) {
	// Use the console to output diagnostic information (console.log) and errors (console.error)
	// This line of code will only be executed once when your extension is activated
	console.log('Congratulations, your extension "helloworld-minimal-sample" is now active!');

	// The command has been defined in the package.json file
	// Now provide the implementation of the command with  registerCommand
	// The commandId parameter must match the command field in package.json
	let disposable = vscode.commands.registerCommand('extension.helloWorld', async () => {
		// The code you place here will be executed every time your command is executed

		const quickpick = vscode.window.createQuickPick();
		quickpick.items = [{ label: 'step 1' }];

		void (await new Promise(resolve => {
				quickpick.onDidAccept(() => {
						console.log(quickpick.selectedItems.map(i => i.label).join(', '));

						if (quickpick.selectedItems.length === 0) return;

						if (quickpick.selectedItems[0].label === 'step 1') {
								quickpick.value = '';
								quickpick.canSelectMany = true;
								quickpick.items = [{ label: 'a' }, { label: 'b' }, { label: 'c' }];
								// If I uncomment this it will fix the bug
								// quickpick.selectedItems = [];
						}
						else {
								resolve();
						}
				});

				quickpick.show();
		}));

		quickpick.hide();
	});

	context.subscriptions.push(disposable);
}

// this method is called when your extension is deactivated
function deactivate() {}

// eslint-disable-next-line no-undef
module.exports = {
	activate,
	deactivate
}

I see this error:

This isn't really something we should be warning about in JS.

[mjbvz]

Note for repro: this requires setting:

        "strict": true,
        "checkJs": true

In your jsconfig

[mjbvz]

Minimal repo:

new Promise(resolve => resolve());

Can be worked around with a type annotation:

/**
 * @type {Promise<void>}
 */
(new Promise(resolve => resolve()));

[mjbvz]

Moving upstream. The current behavior looks expected for strict mode but I agree the experience is not great.

Looks like there is already a quick fix for it though:

Some other ideas:

• Have JS type checking behave differently than normal TS in this case (being inconsistent might be confusing though)
• Add a better error message that mentions using a type annotation

[JacksonKearl]

Hm interesting, I didn't notice the quick fix, and I see I did have:

"js/ts.implicitProjectConfig.checkJs": true,
"js/ts.implicitProjectConfig.strictNullChecks": true,

globally configured.

So I guess this is pretty reasonable, I'd be fine to just close it out.

[andrewbranch]

This is very much something we have tried / are trying / would like to solve; there's an experiment up at #40466 and design notes at #40505. I do think we should probably modify the error message for JS files though. (But it would be better if we could just not error 😕.)

[andrewbranch]

@DanielRosenwasser @rbuckton what should this message even say in JS? We should at least ditch the “did you forget” language, since what we’re asking JS users to do is pretty weird.

[DanielRosenwasser]

Maybe

Expected 1 arguments but got 0. TypeScript may need a hint that the call to 'new Promise()' produces a 'Promise<void>'.

[andrewbranch]

+JSDoc?

Expected 1 arguments but got 0. TypeScript may need a JSDoc hint that the call to 'new Promise()' produces a 'Promise<void>'.

[osaimola]

Hi all, I'm new here and trying to take this one on. Would appreciate some feedback on the pr (especially around localization)
thanks!

[andrewbranch]

@osaimola localization happens automatically after the fact. All you have to do is write the English text in diagnosticMessages.json.

[frank-dspeed]

@DanielRosenwasser @andrewbranch

This confused me:

+JSDoc?

Expected 1 arguments but got 0. TypeScript may need a JSDoc hint that the call to 'new Promise()' produces a 'Promise<void>'.

a new Promise() without a handler or anything in it that calls resolves produces already never and that is correct has the analyzer can not track that it gets resolved some where. it does 100% not return void. new Promise(res=res()) produces void. and even calling res with (undefined) is void. at last out of engine view it translates to the same byte code.

[nicdard]

@andrewbranch Is this issue still open? In case I would like to contribute :)

[andrewbranch]

@nicdard yes, #46637 was on the right track but was then abandoned. If you pick it up, please branch off @osaimola’s WIP or cherry-pick their commit(s) so they get attribution for the work they did.

[nicdard]

Ok I'll try during this weekend, thanks! :)

[jimmywarting]

this error message is very annoying... don't think a jsdoc should even be necessary (as the resolver argument is optional)
In many cases you don't need to resolve with anything. it should just know that passing 0 arguments results with undefined

[fatcerberus]

@jimmywarting The argument is not optional (in a strong-typing sense) for anything other than a Promise<void>. The default inference in the absence of a type hint is Promise<unknown>, hence the error. TS currently has no mechanism to infer Promise<void> automatically.

[frank-dspeed]

@fatcerberus it is missing in general a lot of inference by design but it is the best that works at present i am not sure about the state of flow is it solving some issues does it still exist?

[jimmywarting]

I think it's should be optional... I seen many ppl using pattern like the following where a return value isn't necessary

function sleep (time) {
  return new Promise(resolve => setTimeout(() => resolve(), time))
}
// or
sleep = time => new Promise(resolve => setTimeout(resolve, time))

• I have seen them in api's too where you just want to post/save some data and return a empty 201 - created response
• also every time when you create a async function that don't return anything then you are essentially doing the same thing but with a different way of writing stuff with more syntactic suger.

worth mentioning that i do not write stuff with typescript syntax. i only have checkJS=true turned on
so i do not write stuff like the return value such as Promise<void> or stuff like: function foo():string { ... }
(i only use jsdocs - cuz i hate transpilers and i like my code to be importable across all different kind of environments + it's easier to debug non transformed code... what you write is what you get...)

[fatcerberus]

Right, that’s what this issue aims to solve, so you can write that without complaint. But in general, the argument is not (and should not be) optional if the promise has a type other than void: if I have a Promise<number> then it doesn’t make sense to let me resolve it without an argument, so I’m glad the compiler makes it required in that case.

I don’t think anybody is arguing with you that your examples should work as written; the fact they don’t is due to a design limitation of generic inference that needs to be solved. Making the resolver argument optional would of course fix your examples, but in the process it would compromise type safety for TS programs (or even jsdoc-based ones with checkJs!) using strongly-typed promises. So that’s not a good tradeoff, IMO.