denoland / std

The Deno Standard Library
https://jsr.io/@std
MIT License
3.17k stars 621 forks source link

discussion: runtime type-checking #3888

Open iuioiua opened 11 months ago

iuioiua commented 11 months ago

Most functions/methods in the Standard Library do not check the type-check arguments at runtime. However, some do. Aiming for consistency, this begs the question: should we type-check arguments at runtime or not throughout the Standard Library? This question is mostly important for transpilation of browser-compatible modules for the browser. E.g. Fresh islands. There are 3 prominent possible approaches and arguments.

1. Type-check all modules The benefit is that all code, when transpiled, is type-guarded. This is nice but would incur a considerable engineering cost, especially if done codebase-wide. The risk is if this is done but not needed or demanded, that would mean a substantial ongoing engineering cost. This is what Node does. All code would also become dependent on std/assert.

2. Don't type-check any modules (for now) It's possible that the demand for type-checking is zero or near zero. In that case, the main benefit of this approach is that it could avoid needless engineering costs. It's also possible that, in the future, there will be a demand for type-checking expressed by the community. This approach would allow us to re-evaluate while being more informed in the future. We would have to clearly state, somewhere in the README, that no type-checking is done, and this fact should be considered for those transpiling code for the browser.

3. Only type-check web-compatible modules This approach meets in the middle of the other two. An engineering cost would be incurred on browser-compatible modules but not on the remainder of the modules.

I'm most in favour of option 2. It's simple and avoids wasting any time on possibly needless type-checking, allowing us to make a more informed decision in the future. It'd require us to perform some cleanups around the codebase, but that wouldn't take too long.

Community, what do we think? Are there other unexplored options? Please let us know.

kt3k commented 11 months ago

It's possible that the demand for type-checking is zero or near zero.

I disagree with this. Runtime type-checking is not a kind of feature which users immediately need when they start using these APIs, but it's a feature which helps debugging very difficult bugs that were thrown from deep in the stack. The lack of demand for it can't be a reason for removing them in my view.

TypeScript is not strictly typed language. There always be chances for variables to be in wrong types in runtime even when the users completely follow the TS types. (Many builtin APIs return any type, typically json related APIs. Those values are not validated by TS typing system at all, and these can be passed to any APIs with absurd types. Runtime type checking helps preventing to throw very enigmatic error messages when such situation happens)

jcayzac commented 11 months ago

Personally I would love being able to enable input validation in all APIs using a flag, + being able to query that flag (through a new property in the Deno object maybe) so that I can also have my own code enforce it (e.g. using zod).

jollytoad commented 11 months ago

I'm in two minds about this, as @kt3k says, TS is not robust enough to rule out the possibility of incorrect types being passed, and runtime checking could be very useful in development and testing. On the flip-side, this required duplicating types (static and runtime), and inconsistent/conflicting runtime checks could be worse than having none.

I've always thought that runtime checking if desired could be something that can be optionally generated from the static types, ie function wrapper that check, but I'm not aware of any tools that currently attempt that.

Whatever the outcome I think we need to be able to opt-in or out of the runtime checks, which could possibly be done using import mapping (eg. an alternative set of no-op assert modules).

I think it should be a post-1.0 thing at least, esp if opt-in/out is possible.

scarf005 commented 11 months ago

I've always thought that runtime checking if desired could be something that can be optionally generated from the static types, ie function wrapper that check, but I'm not aware of any tools that currently attempt that.

As far as i know, some libraries try this approach:

jollytoad commented 11 months ago

Thanks I've not seen typia before, although I'm aware of runtime type checking libs such as zod (and ts-to-zod), typebox, io-ts etc. What I'm not aware of is any attempt to take a module, and augment it (either directly or with wrapper fns) with checks. For example, you could using ts-to-zod to generate all of the zod runtime schemas from a module and then generate a 'wrapper' module with fns that validate the arguments using the zod schemas before calling the original fn, and maybe even validating the return type. This kind of thing could be generically useful beyond std, to check your entire app, and/or it could be a way to automatically produce a checked alternative of std without the ongoing maintenance effort of the runtime checks.

jtoppine commented 11 months ago

Would runtime type checking force typescript semantics to JS only users?

JS is dynamic by nature, and duck typing can be usefull in various situations. I should be able to pass an object that looks/acts like an instance of the thing although it doesn't exactly pass instanceof check. That may be a illegal in TS culture, but not unheard of in JS.

If I create a full-blown DIY implementation of let's say Request object that has identical methods and properties, I would think I should be able to pass it to std methods without issue. Would runtime type checking prevent that? I guess one could class MyRequest extends Request, so I'm not exactly sure whether that's a problem, though.

Another example is custom DOM implementations. Although IIRC Deno/std currently doesn't have DOM or anything working with DOM, but it's perhaps likely to have in the future.

I do actually have and rely quite heavily on a custom pure Javascript implementation of XML DOM, HTMLDocument, ClassList, shadow roots and all. It can't extend native classes since there are no such things in Deno, and not even sure if that would be a good idea in deno or the browser. It's pretty nice that I can pass that custom DOM to various libraries and utilities that work on the DOM. The libraries might be made to work on native browser DOM, but work nonetheless on deno server code too if the custom DOM have the API methods they expect. That wouldn't work if they would typecheck on runtime against a specific module or built-in class.

Admittedly talking hypotheticals here, and I'm not super confident I understand the topic deeply enough to have a strong opinion for or against. Just worried a bit about the potential concequences of shoehorning TS style typechecking to JS runtime code, is all.

About runtime argument checking in general, I would think a hardline policy of always checking arguments ("type checking", or a more dynamic "validity check") is a bad idea. It does affect performance, and makes code very verbose with all the checking boilerplate. For non-performance centric code, and for cases where the cost or likelyhood of invalid arguments is high, sure it's nice to have some input checks.

jollytoad commented 11 months ago

TS is very much based on structural/duck typing. Many existing runtime checkers (eg. zod) validate the shape of an object rather than it's instance, as the primary use case is to validate deserialized JSON data.

To use your examples, Request and other DOM objects are defined purely as types in TS (ie. interfaces or type aliases as opposed to classes), so that would imply checking by structure rather than instanceof.

You touch on a very good point though, how deep would runtime checking go? Assertions of just primitive types? That an expected object is just an object? Or do we deeply check the object has every declared property and recursively? And what about functions and methods, we can never be sure at runtime what the signature is of these.

It certainly is a rabbit hole, and without opt-out ability a potential performance one at that.

iuioiua commented 11 months ago

Yes, it can be a rabbit hole. For now, I suggest we remove any few instances of runtime type-checking until we have a fleshed-out solution. At least, the Standard Libary would be consistent in that way.

Leokuma commented 10 months ago

Can we be inconsistent and typecheck arbitrarily? We could typecheck the most error prone functions and functions that can lead to enigmatic errors like syscalls or something.

On a side note, I think it would be worth considering adding something like Zod to std.

iuioiua commented 10 months ago

Can we be inconsistent and typecheck arbitrarily? We could typecheck the most error prone functions and functions that can lead to enigmatic errors like syscalls or something.

To be clear, this issue mainly pertains to type-checking function arguments against their explicitly annotated type. E.g. checking value is a string in fn(value: string). Checking values, according to their type, is already done throughout the Standard Library and is a good thing that we should keep. E.g. checking value is within a particular range in fn(value: number).

On a side note, I think it would be worth considering adding something like Zod to std.

This has been brought up before in Discord: https://discord.com/channels/684898665143206084/775393009981849600/1192916616728563712

Perhaps, it's worth opening an issue for discussion.

scarf005 commented 3 months ago

I'm for not type-checking all modules. unless it's REPL, it's highly unlikely modules are used without type checking. even in JS it can be easily enabled via // @ts-check.

kt3k commented 3 months ago

If a value is typed as any, static type checking doesn't work, and there are many builtin APIs which return any (JSON.parse(), response.json(), etc).