Include unstable APIs type declarations even if --unstable flag is not present

[bartlomieju]

As per title; I suggest we change the behavior of --unstable flag to only have impact during runtime.

Currently type checking includes different declaration files depending whether --unstable flag is present or not. This creates friction for users with not much benefit (IMO). We have a system that rewrites messages coming from TSC about non-existent types (by using UNSTABLE_DENO_PROPS list in cli/diagnostics.rs) and suggests to run again with --unstable flag, however that system is not completely reliable and some errors don't return any meaningful suggestions. Additionally, the system requires us to manually sync which APIs are considered unstable and as can be seen in #12723 we sometimes forget to update the list. One might argue that we should update the system and/or add more checks that will ensure that the lib.deno.unstable.d.ts and UNSTABLE_DENO_PROPS list is kept in sync, but IMO this will lead to even more complexity, while still not being able to catch all situations where --unstable flag is missing (ergo still showing non-helpful message to users).

Another reason is that some deno_std modules use unstable APIs (denoland/deno_std#1539, or half of deno_std/node), and even though users might not use any of the unstable APIs, they are still required to pass --unstable flag. Again, one might argue that we should refactor deno_std in a way that splits modules depending if they use unstable APIs or not (eg. fs/mod.ts and fs/mod.unstable.ts). For me this adds more maintenance burden as well as increases users friction and discoverability of the modules.

So I propose we always include type declarations for unstable APIs during type checking. To better signal to users that they are depending on unstable APIs we might leverage DiagnosticTag::DEPRECATED in the LSP, which would strike-through the API name, prompting users to hover over the API, which would show the docstring that starts with the description that the API is not yet stable.

[ry]

I’m in favor of this. It’s not particularly dangerous - we still have the ability to change unstable APIs. It’s just users will get runtime errors instead of type errors.

[nayeemrmn]

we still have the ability to change unstable APIs. It’s just users will get runtime errors instead of type errors.

No, the difference is that when we break an unstable API (change the argument types, remove it altogether, etc), the type-check breakage could previously only affect users using --unstable but after this change, it could affect users not using --unstable as well. Since you would no longer have to pass --unstable to import a TS module that uses an unstable API that would be subject to type checking. It's a big deal, we need to keep this system to properly silo off --unstable code.

However, I think this would be a great thing to consider if #11970 is implemented. Things like this (more approachable) and #11340 (not as necessary?) could be looked at from a new lens after #11970 and we should consider even doing it ahead of 2.0. Largely people have been okay with breakages as long as they were under their control.

[dsherret]

I think @nayeemrmn makes a good point here—adding even optional properties to an interface could potentially break something somewhere. It's true that people right now would probably need to deal with this anyway with users using --unstable so maybe this is not a big deal though?

Another issue is that including these unstable types might encourage users to rely on unstable more.

I don't think we would be considering this issue if #11970 were implemented already. Perhaps we should resolve that first before tackling this one?

Another reason is that some deno_std modules use unstable APIs (denoland/deno_std#1539, or half of deno_std/node), and even though users might not use any of the unstable APIs, they are still required to pass --unstable flag. Again, one might argue that we should refactor deno_std in a way that splits modules depending if they use unstable APIs or not (eg. fs/mod.ts and fs/mod.unstable.ts). For me this adds more maintenance burden as well as increases users friction and discoverability of the modules.

One option for deno_std would be to create an internal "deno unstable" module that uses unstable APIs without type checking, but offers a type safe API to the rest of deno_std. Then if code in deno_std wanted to use an unstable API they could import this module. The downside would be no type checking in the module's implementation, but we could just rely on unit tests.

[bartlomieju]

Thanks for opinions @nayeemrmn, @dsherret. If we were to go with #11970 then this issue will most likely become irrelevant.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.