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

Change return type of valueOf to prevent assignment #178

Open
mrcljx opened this issue Aug 16, 2022 · 2 comments
Open

Change return type of valueOf to prevent assignment #178

mrcljx opened this issue Aug 16, 2022 · 2 comments

Comments

@mrcljx
Copy link

mrcljx commented Aug 16, 2022

Currently the return type of valueOf is never. This is correct
as it will never return a value but always throw.

The issue is that never actually behaves a lot like any.

The never type is a subtype of, and assignable to, every type;
however, no type is a subtype of, or assignable to, never (except
never itself). Even any isn’t assignable to never.

const v = instant.valueOf()
console.log(v + 1) // no problem

It further means that the following snippet will not raise an error
at compile time (while it will at runtime).

type MyInterface = { valueOf: () => number }
const value: MyInterface = Temporal.Now.instant()

A few options of what one could do to make TypeScript point out
the issue here:

  1. Mark valueOf as private,
  2. Make valueOf optional,
  3. Make valueOf return unknown, undefined, or a unique symbol.

I added an exploration of these options in a playground.

@shicks
Copy link

shicks commented Aug 16, 2022

I updated your playground example to also call valueOf() directly. Interestly, I couldn't reproduce the "unreachable code" error with the current version (can you link to an example of this?).

Here are my thoughts on the three options:

  1. Making valueOf private seems promising, assuming it's reasonable to be a class. Obviously it's not possible for an interface.
  2. Making valueOf optional seems somewhat unsatisfying. One possible consideration is that if subclasses are possible, then they can override an optional method with a required method (though it would still have to return never for covariance).
  3. Note that never|x is the same as just x, since never is the union identity. So I don't think this is a good option - it's also the only option of the three that permits direct calls to valueOf without any complaint at all.

@mrcljx
Copy link
Author

mrcljx commented Aug 16, 2022

I couldn't reproduce the "unreachable code" error with the current version (can you link to an example of this?).

You are right. I must have imagined this working, but it actually does not.

Note that never|x is the same as just x

You are right there well. I meant to actually remove that suggestion. unknown or unique symbol would be the remaining options to at least prevent assignment and still making the value somewhat unusable.

edit: Updated issue summary to reflect above discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants