Skip to content
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

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

Open
JacksonKearl opened this issue Oct 28, 2021 · 10 comments · May be fixed by #46637
Open

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

JacksonKearl opened this issue Oct 28, 2021 · 10 comments · May be fixed by #46637

Comments

@JacksonKearl
Copy link

@JacksonKearl JacksonKearl commented Oct 28, 2021

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:
image

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

@mjbvz
Copy link
Contributor

@mjbvz mjbvz commented Oct 29, 2021

Note for repro: this requires setting:

        "strict": true,
        "checkJs": true

In your jsconfig

@mjbvz
Copy link
Contributor

@mjbvz mjbvz commented Oct 29, 2021

Minimal repo:

new Promise(resolve => resolve());

Can be worked around with a type annotation:

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

@mjbvz mjbvz transferred this issue from microsoft/vscode Oct 29, 2021
@mjbvz mjbvz removed their assignment Oct 29, 2021
@mjbvz
Copy link
Contributor

@mjbvz mjbvz commented Oct 29, 2021

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:

Screen Shot 2021-10-28 at 6 40 23 PM

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
Copy link
Author

@JacksonKearl JacksonKearl commented Oct 29, 2021

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
Copy link
Member

@andrewbranch andrewbranch commented Oct 29, 2021

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
Copy link
Member

@andrewbranch andrewbranch commented Oct 29, 2021

@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
Copy link
Member

@DanielRosenwasser DanielRosenwasser commented Oct 29, 2021

Maybe

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

@andrewbranch
Copy link
Member

@andrewbranch andrewbranch commented Oct 30, 2021

+JSDoc?

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

@osaimola
Copy link

@osaimola osaimola commented Nov 2, 2021

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
Copy link
Member

@andrewbranch andrewbranch commented Nov 2, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

5 participants