Repository navigation
Unhelpful "Did you forget to include 'void' in your type argument to 'Promise'" in JS file #46570
Description
TS Template added by Matt Bierner (@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.
Activity
mjbvz commentedon Oct 29, 2021
Note for repro: this requires setting:
In your jsconfig
mjbvz commentedon Oct 29, 2021
Minimal repo:
Can be worked around with a type annotation:
mjbvz commentedon 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:
Some other ideas:
JacksonKearl commentedon Oct 29, 2021
Hm interesting, I didn't notice the quick fix, and I see I did have:
globally configured.
So I guess this is pretty reasonable, I'd be fine to just close it out.
andrewbranch commentedon 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 commentedon Oct 29, 2021
Daniel Rosenwasser (@DanielRosenwasser) Ron Buckton (@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 commentedon Oct 29, 2021
Maybe
andrewbranch commentedon Oct 30, 2021
+JSDoc?
osaimola commentedon 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 commentedon Nov 2, 2021
Osa (@osaimola) localization happens automatically after the fact. All you have to do is write the English text in
diagnosticMessages.json.frank-dspeed commentedon Mar 29, 2022
Daniel Rosenwasser (@DanielRosenwasser) Andrew Branch (@andrewbranch)
This confused me:
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 commentedon Mar 31, 2022
Andrew Branch (@andrewbranch) Is this issue still open? In case I would like to contribute :)
andrewbranch commentedon Mar 31, 2022
Nicola Dardanis (@nicdard) yes, #46637 was on the right track but was then abandoned. If you pick it up, please branch off Osa (@osaimola)’s WIP or cherry-pick their commit(s) so they get attribution for the work they did.
nicdard commentedon Apr 1, 2022
Ok I'll try during this weekend, thanks! :)
jimmywarting commentedon Sep 8, 2022
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 commentedon Sep 13, 2022
Jimmy Wärting (@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 isPromise<unknown>, hence the error. TS currently has no mechanism to inferPromise<void>automatically.frank-dspeed commentedon Sep 13, 2022
Bruce Pascoe (@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 commentedon Sep 13, 2022
I think it's should be optional... I seen many ppl using pattern like the following where a return value isn't necessary
worth mentioning that i do not write stuff with typescript syntax. i only have
checkJS=trueturned onso 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 commentedon Sep 13, 2022
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 aPromise<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.