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

Removing unstable types incorrectly suggests running with --unstable #23079

Closed
irbull opened this issue Mar 26, 2024 · 8 comments · Fixed by #23092
Closed

Removing unstable types incorrectly suggests running with --unstable #23079

irbull opened this issue Mar 26, 2024 · 8 comments · Fixed by #23092
Labels
bug Something isn't working

Comments

@irbull
Copy link
Contributor

irbull commented Mar 26, 2024

Version: Deno 1.41.3

If you run deno check on the following code, it will complain about the unstable APIs. If you specify --unstable you get a warning that you don't need --unstable AND a suggestion to add it.

/// <reference no-default-lib="true" />
/// <reference lib="deno.ns" />

Deno.dlopen("path/to/lib", {});

If you run deno check

$ deno check --unstable main.ts                                                                                                                                                                                                        (base)  Mon Mar 25 19:40:51 2024
⚠️  The `--unstable` flag is not needed for `deno check` anymore.
Check file:///Users/irbull/git/deno/unstable/main.ts
error: TS2551 [ERROR]: Property 'dlopen' does not exist on type 'typeof Deno'. Did you mean 'open'? 'Deno.dlopen' is an unstable API. Did you forget to run with the '--unstable' flag, or did you mean 'open'? If not, try changing the 'lib' compiler option to include 'deno.unstable' or add a triple-slash directive to the top of your entrypoint (main file): /// <reference lib="deno.unstable" />
Deno.dlopen("path/to/lib", {});
     ~~~~~~
    at file:///Users/irbull/git/deno/unstable/main.ts:4:6

It tells you to add --unstable.

I was hit by this in a Fresh project where I wasn't aware of the triple slash directives in the boilerplate code.

@irbull
Copy link
Contributor Author

irbull commented Mar 26, 2024

Also, deno.unstable in the settings.json isn't honoured in this case. The Deno Tooling will complain about the unstable API usage.

@dsherret dsherret changed the title no-default-lib + deno.ns directives removes unstable APIs from deno check Removing unstable types incorrectly suggests running with --unstable Mar 27, 2024
@dsherret dsherret added the bug Something isn't working label Mar 27, 2024
@dsherret
Copy link
Member

no-default-lib + deno.ns directives removes unstable APIs from deno check

This previous title is actually correct behaviour. The directives take precedence (see #20420)

It saying to run with --unstable is out of date information though. I'll open a PR in a few mins.

@irbull
Copy link
Contributor Author

irbull commented Mar 27, 2024

What do you think about the tooling? Even with unstable:true VSCode reported errors. I guess the directives take precedence here too, but it's rather subtle, especially since I wasn't even aware of of those directives as they came from some boilerplate Fresh code. I wonder how we make it more obvious why the errors are being reported.

Screenshot 2024-03-26 at 8 11 20 PM

@irbull
Copy link
Contributor Author

irbull commented Mar 27, 2024

Maybe a better question, is the deno.unstable option still needed (https://github.com/denoland/vscode_deno/blob/main/package.json#L477)? If it's not required for deno check, is there a reason the LSP needs it for type checking?

@bartlomieju
Copy link
Member

I believe this setting is no longer user. @nayeemrmn can we remove it now?

@nayeemrmn
Copy link
Collaborator

We still use it for adding --unstable when running tests via the explorer

@irbull
Copy link
Contributor Author

irbull commented Mar 27, 2024

Maybe we should update the description of the setting to reflect that. denoland/vscode_deno#1095

@dsherret
Copy link
Member

I guess the directives take precedence here too, but it's rather subtle, especially since I wasn't even aware of of those directives as they came from some boilerplate Fresh code.

Yeah, the directives take precedence.

I wonder how we make it more obvious why the errors are being reported.

It's difficult to do that. Probably #20420 would help eliminate most of the confusion here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants