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

Interfaces that extend 'Symbol' cannot be used as 'symbol' type directly #46956

Closed
yusufkandemir opened this issue Nov 30, 2021 · 7 comments
Closed
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@yusufkandemir
Copy link

yusufkandemir commented Nov 30, 2021

Bug Report

🔎 Search Terms

extends Symbol
extends Symbol index key
extending Symbol
InjectionKey
cannot be used as an index type

🕗 Version & Regression Information

⏯ Playground Link

Playground link with relevant code

💻 Code

// Reference for InjectionKey: https://github.com/vuejs/vue-next/blob/2d4f4554349db6b07027d0c626f56c48d0233f67/packages/runtime-core/src/apiInject.ts#L6
interface InjectionKey<T> extends Symbol {}

interface SomeInterface { foo: string }
const key: InjectionKey<SomeInterface> = Symbol()

const records: Record<string | symbol, any> = {};

records['a-string'];     // Works
records[Symbol()];       // Works
records[key];            // Doesn't work
records[key as symbol];  // Works

// Assignability tests
const _key1: symbol = key;           // Doesn't work
const _key2: symbol = key as symbol; // Works

// Casting tests

key as symbol; // Works        (Expected)
key as string; // Doesn't work (Expected)
key as number; // Doesn't work (Expected)
// ...

const sym = Symbol();

sym as symbol; // Works        (Expected)
sym as string; // Doesn't work (Expected)
sym as number; // Doesn't work (Expected)
// ...

🙁 Actual behavior

Interfaces that extend Symbol(e.g. InjectionKey) can't be used as symbol directly although they can be explicitly cast.

🙂 Expected behavior

Interfaces that extend Symbol can be used as symbol, which includes being used as index types.

@whzx5byb
Copy link

A wrapper object type(Symbol, Number, String...) is not assignable to the corresponsive primitive type(symbol, number, string...). And the doc says you should avoid using them in practice.

In your example, consider making the InjectionKey as a intersection type.

type InjectionKey<T> = symbol & T;

@yusufkandemir
Copy link
Author

@whzx5byb thanks for the info and suggestions 👏.

type InjectionKey<T> = symbol & T;

wouldn't be the same as

interface InjectionKey<T> extends Symbol {}

T shouldn't be a possible value.

So, it would be like

type InjectionKey<T> = symbol;

then the type becomes primitive, which makes extracting the generic parameter type impossible.

I also found that the docs don't recommend using a generic type parameter with an unused type parameter, see here. So, this means the code I shared is wrong in many ways. But, I think it's the only way possible to create symbols that are paired with a specific type.

See this playground for a use case, with two approaches.

At this point, it looks like it's against the best practices, but the only way to solve a particular problem, at least the people at Vue, and also me after digging through this. So, I guess the maintainers could just close the issue because it's against their recommendation(which is fine), provide a legit alternative solution to this problem(it would be fantastic), or search for ways to create a legit way if it doesn't exist yet(i.e. they are willing to accept a feature request about this).

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Nov 30, 2021
@RyanCavanaugh
Copy link
Member

Yeah, this code just isn't doing what you think it's doing. The type Symbol is an object type, and object types aren't valid object keys (the same reason foo[{ }] is not allowed). The same problem is present with number / Number, etc

@typescript-bot
Copy link
Collaborator

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@pikax
Copy link

pikax commented Dec 8, 2021

@yusufkandemir the InjectionKey on vue 3 is to be used only with provide/inject

The reason that extends Symbol is to make sure the user is using with a unique symbol to guarantee the dependency at runtime.

If someone is using InjectionKey<T> to direct access a property (like the example on the issue) that's is not supported by vue.

Since this interface is only to be used with provide/inject in vue I don't think there's an issue with the current implementation on vue 3, since the goal is to have a typed safe interface for inject/provide.

@yusufkandemir
Copy link
Author

@pikax I appreciate the explanation. The main idea behind this was basically to have an appInject helper, which is still possible to achieve, and not directly related to this issue. But, when playing around with the idea, I came across some problems and dived into how everything works, how TS behaves, etc., and decided to open this issue because it seemed like a bug.

I guess you are mainly referring to this:

So, this means the code I shared is wrong in many ways.

which is correct in terms of "being wrong according to the TS best practices".
And, it is followed by

But, I think it's the only way possible to create symbols that are paired with a specific type.

and similar supporting sentences.

So, with the last answer I received from Ryan Cavanaugh(don't want to bother with a ping), the situation became clearer(I hope I understood it correctly 😛). The current implementation in Vue 3 is correct, because it just works and it's clean to use and understand, and there is no other way possible to achieve this, even though it looks like it's against TS best practices. But, I think it's safe to say that if this is the intended behavior of TS, and there is no other way of achieving such a thing (both of which seems to be the case), we can't really say the current implementation on Vue 3 is against the TS best practices. A best practice becomes invalid when it completely blocks us from achieving something. By the way, I love your work ❤️

@pikax
Copy link

pikax commented Dec 8, 2021

Since vue also need to allow defining provide/inject through the object API, is worth to investigate a bit further, based on the @whzx5byb it works but adds the possibility of assigning T as InjectionKey<T> which is not correct.

Based on that I think is save to do something like:

declare const InjectionSymbol: unique symbol
type InjectionKey<T> = symbol & { [InjectionSymbol]: T };

//@ts-expect-error it cannot be the same type
const foo = {foo: false} as InjectionKey<{foo: false}>

The only issue, it works now but will it work in the future or will typescript support this type in the future? For example PropertyKey<T> typed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

5 participants