TS1108 "problem" showing on pure Javascript (node) file - Erroneous Error Flag

[robertblum-psi]

Issue Type: Bug

PROBLEMS pane shows:
[JS] file.js (1)
A 'return' statement can only be used within a function body. ts(1108) [400,5]

My node Javascript file is statements that call defined functions. These statements have an if statement that executes a return statement when true. The program is Javascript, and recognized as such by editor. There is no typescript in the environment. This bug appeared immediately after VS Code updated to February 2022 version (1.65.2). It did not appear in the previous version with the exact same file. The program runs properly on the target when run.

UPDATE: I have installed 1.64.2 to remediate issue. The problem does not occur down-level.

The program looks like this:

#!/usr/bin/env node
statements and function definitions
if ( x == y)
return
statements

A Typescript error should not show for a Javascript file.

VS Code version: Code 1.65.2 (c722ca6c7eed3d7987c0d5c3df5c45f6b15e77d1, 2022-03-10T14:33:55.248Z)
OS version: Windows_NT x64 10.0.19043
Restricted Mode: No

System Info

Item	Value
CPUs	Intel(R) Core(TM) i7-6770HQ CPU @ 2.60GHz (8 x 2592)
GPU Status	2d_canvas: enabled gpu_compositing: enabled multiple_raster_threads: enabled_on oop_rasterization: enabled opengl: enabled_on rasterization: enabled skia_renderer: enabled_on video_decode: enabled vulkan: disabled_off webgl: enabled webgl2: enabled
Load (avg)	undefined
Memory (System)	31.89GB (23.34GB free)
Process Argv
Screen Reader	no
VM	0%

Extensions (11)

Extension	Author (truncated)	Version
Bookmarks	ale	13.2.4
bracket-pair-colorizer	Coe	1.0.62
html-preview-vscode	geo	0.2.5
vscode-pull-request-github	Git	0.38.1
ibm-jcl	kel	0.11.0
remote-wsl	ms-	0.64.2
rexx	nex	0.0.1
deepdark-material	Nim	3.3.0
ayu	tea	1.0.5
pdf	tom	1.2.0
vscode-extension-for-zowe	Zow	1.22.0

(17 theme extensions excluded)

A/B Experiments

vsliv368cf:30146710
vsreu685:30147344
python383cf:30185419
vspor879:30202332
vspor708:30202333
vspor363:30204092
pythonvspyl392:30443607
pythontb:30283811
pythonvspyt551cf:30345471
pythonptprofiler:30281270
vshan820:30294714
vstes263:30335439
vscorecescf:30445987
pythondataviewer:30285071
vscod805:30301674
pythonvspyt200:30340761
binariesv615:30325510
bridge0708:30335490
bridge0723:30353136
vsaa593:30376534
vsc1dst:30438360
pythonvs932:30410667
wslgetstarted:30449410
vsclayoutctrt:30451275
dsvsc009:30440023
pythonvspyt640:30450904
vscscmwlcmc:30438804
vscgsvid2:30447481
pynewfile477:30450038

[mjbvz]

@sandersn Looks like node does allow top level returns so we may want to exclude this from the default JS syntax checks

@robertblum-psi This was added in 1.65, see https://code.visualstudio.com/updates/v1_65#_syntax-error-reporting-in-javascript-files for details and how you can disable this

[robertblum-psi]

@mjbvz

@robertblum-psi This was added in 1.65, see https://code.visualstudio.com/updates/v1_65#_syntax-error-reporting-in-javascript-files for details and how you can disable this

Javascript and Nodejs are not Typescript, and have their own conception of typing that is often radically different than Typescript's. It is a bad idea to enable this for Javascript by default. This change needs to be reverted.

[mjbvz]

@robertblum-psi This is not related to TS vs JS

These new checks enabled for JavaScript assumed that top level return statements are always a mistake (because in most JS environments they are). However it looks like node sometimes allows them, so we likely don't want to enable this specific check by default

Worth noting that returns are still invalid in node if the file is a module and using them goes against the JS spec afaik

[robertblum-psi]

@mjbvz
I consider the setting "javascript.validate.enable" to be misleading. Because it turns on Typescript-like validation, it ought to be named javascript.typescriptvalidate.enable instead.

[robertblum-psi]

@mjbvz:

... assumed that top level return statements are always a mistake (because in most JS environments they are). However it looks like node sometimes allows them...

Node always does AFAIK. JS and Nodejs are very flexible.

It occurs to me that some thought then ought to be given to a node flavor of Javascript, like there is one for REACT. I am not advocating this. I would much rather that there be no Typescript checks on Javascript code, full stop.

[fatcerberus]

The checks that have been enabled are not typing related (that’s what checkJs is for). They are (intended to be) for semantic/syntax errors that will cause an issue at runtime but were too complex to detect in the parser. For example, previously this wasn’t caught as an error in JS files despite being a syntax error at runtime:

const foo = {
    foo = 1,
    bar = 2,
}

now it is. These kinds of checks are a good thing.

[MartinJohns]

I would much rather that there be no Typescript checks on Javascript code, full stop.

Does the mentioned setting not work for you?

[RyanCavanaugh]

I consider the setting "javascript.validate.enable" to be misleading. Because it turns on Typescript-like validation, it ought to be named javascript.typescriptvalidate.enable instead.

There seems to be some confusion here. The intent of the functionality is to error on illegal JavaScript constructs. This is implemented by using the TypeScript checker and filtering for those particular things which are also illegal in JS.

Exposing the implementation details of how this happens to the user is, of course, intentionally not done.

[robertblum-psi]

@RyanCavanaugh
@fatcerberus

Exposing the implementation details of how this happens to the user is, of course, intentionally not done.

Um... It clearly outputs a TS(1108) error. That is a Typescript error. You appear to be using a typescript parser somewhere on JavaScript code. And, because of this one error, I now realize that many (if not all) JS errors VS Code flags are presented with Typescript error messages.

Does the mentioned setting not work for you?

I down-leveled to 1.64.2 to get work done. And...

The checks that have been enabled are ... (intended to be) for semantic/syntax errors that will cause an issue at runtime but were too complex to detect in the parser.

If this is so, why would I want or need to enable or disable setting? Such things should be seamless. If they are "too complex for the parser", fix the parser. (Or maybe, do nothing.)

Philosophically, I have no problem with fixing the very very very rare occurrence of something that fails the syntax checker when I run my program. I occasionally had to do that even when I used Eclipse. Good programmers test before shipping, so where is the need for this feature? Your solution feels error prone (proven) and like a solution searching for a problem (or a very low priority one). Perfect is the enemy of good. What I had before was good. No, it was very good.

Which brings us back to the setting: My 2 cents is, if the setting is new, set it to disabled and advertise the feature. If the setting existed and was set to false in previous releases and is now set to true, set it back to false by default.

[RyanCavanaugh]

I now realize that many (if not all) JS errors VS Code flags are presented with Typescript error messages.

This is correct; the JS intellisense experience (completions, navigation, errors, hover info, outlining, refactoring, etc) is all powered by the TS engine and they do share error codes. This has been the case since VS Code launched.

Regarding this feature, user feedback was broadly consistent that people didn't like not seeing error squiggles on syntax errors, since it's generally easier to just fix these things in the editor than wait for the script to fail to load. We launched this feature with a very constrained set of errors that we thought would 100% of the time be runtime syntax errors, but overlooked this case.

VS Code has evolved to have many new highly-requested features over the years. This one, as you can see, shipped with a bug, which we'll fix shortly. New features may appear again in the future based on user feedback; if this isn't compatible with your workflow/worldview, I would recommend disabling updates entirely.

[robertblum-psi]

@RyanCavanaugh

I think we may be misunderstanding each other. I believe I became mislead by the advice on javascript.validate.enable chicken switch. It appears to be on in 1.64.2! At least, it is on now when I looked in settings, so it must be a feature I was otherwise okay with. I never manually enabled it, so it must have been enabled from inception. If this is the case, you are already fixing my bug, namely the return statement being erroneously flagged as invalid. Thank you.

If javascript.validate.enable got enabled by installing 1.65.2, then I do have an an additional issue. If javascript.validate.enable was disabled before this update, it should have remained disabled.

It is a best practice to advertise a new feature, rather than turning it on without permission. Of course, there are exceptions, like security features, or enhancements that won't surprise the user or impede workflow. Is this one the latter? If it weren't for the return bug.... maybe?

New features may appear again in the future based on user feedback; if this isn't compatible with your workflow/worldview, I would recommend disabling updates entirely.

I remember when an update broke GIT. I down-leveled one release and "disabled updates entirely". Ok, I set it to manual. GIT not working in the IDE (when it worked adequately previously) wasn't compatible with my workflow or worldview. I set it to auto the month I found it fixed.

[xmedeko]

Maybe it would be nice to have some special form of comment to disable such errors for the specific line. E.g. I have tried on the return line comments line @ts-ignore but the error is still there. (Idela would be something like // @ts-ignore-1108 or // @ts-ignore-no-return etc.

// @ts-nocheck works for the whole file, thought. (But I would like to have rest of JS checking.)

[MartinJohns]

@xmedeko See #19139.

[xmedeko]

@MartinJohns Nice, but even the current // @ts-ignore does not work for this "return ... ts(1108)" error.

[robertblum-psi]

Since we are talking about a tslint function, perhaps a rules property should be surfaced in settings: js-rules?

[sandersn]

Note that return is illegal inside modules, so node complains about it in module.mjs:

function container() {
}
return

I must have mistakenly only tested modules for this error.

Edit: reinforced by the fact that it was the very last test case of grammar errors, in the final section labelled "other weirdness".

[robertblum-psi]

@sandersn I see "fix available". When does that mean it will show up in a VS Code version update? June? Or do I need to download some sort of hot fix?

[sandersn]

typescript@next has the fix now. @mjbvz 's vscode-typescript-next extension makes it easy to use if you don't want to fiddle with settings.

[corngood]

I just came across this problem. If either checkJs or @ts-check are on, it still flags it (isPlainJsFile returns false). That seems wrong to me, but maybe I'm missing something.