-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
Language services for keyof types #11997
Comments
Had a discussion about this with @sandersn and the conclusion is that this wouldn't be easy. For a simplified example: interface I { x: number; y: number; }
const key: keyof I = "x"; The problem is that the type of This is sort of similar to #12687 in that the problem involves information being destroyed during type resolution. A potential solution would be to have the literal types in Related: #1579, because |
Is that even necessary? Do we really care about the type of Say I want to Rename interface I { t: number; y: number }
const key: keyof I = "t"; To achieve this, you don't need to know what Find refs is maybe less well defined? If you could find the literal
A scenario I would really want to light up when finding refs is my initial post: // Your usual pluck def.
function pluck<T, K extends keyof T>(xs: T[], prop: K): T[K][];
class Thing { name: string; }
let x = pluck(things, "name");
// Find all Thing.name ^ I think this should be doable because during compilation you have to validate that the literal |
The type of |
I see! let a: keyof I; // Immediately expanded to a: "x" | "y"
let b: typeof a | "both"; // b: "x" | "y" | "both";
b = "x"; // ok because "x" is in the union type, no direct link with keyof I. I have an implementation idea: what if you keep the type system as is but add an annotation on types literals at expansion time to indicate where they're from. let a: keyof I; // a: "x" [keyof I] | "y" [keyof I]
let b: typeof a | "both"; // b: "x" [keyof I] | "y" [keyof I] | "both";
b = "x"; // literal "x" matches literal type "x" annotated with keyof I Tricky cases are then stuff like: interface Vec2 { x: number; y: number }
interface Vec3 { x: number; y: number; z: number }
type T = keyof Vec2 | keyof Vec3;
// T = "z" [keyof Vec3] | "y" [??] | "x" [??] When merging the unions, it's unclear to me what you should do with the annotations: drop them? combine them? More than a technical issue, it's a spec problem. When renaming My opinion is that you should drop the annotation (if they differ) when combining and not care. The common use case works and there's no perfect solution for edge cases. At least the compiler will catch errors during compilation. Language services for |
Does not seem that there is much we can do at the time being. this is rather a design limitation with the current system. |
Just saying: this is a hard hit into the At least, I feel like you should tag this "Revisit" or something and keep it open until you find a design that can support it. Closing because it's hard to do now doesn't feel good/right. |
This is dependent on the implementation of the compiler. if we did change the representation of keyof to be a different kind of types (instead of a union), then possibly we would be able to revisit this. |
@mhegazy I don't have nearly the same knowledge of TS internals as you, but a type different from a union looks like a lot of extra complexity (to work with existing union, intersection, narrowing, etc.) What about the design I suggested in my comment just above? |
We rely heavily on the identity of the literal types to be able to narrow unions efficiently. that means a literal type We do some form of tagging, by associating aliases with unions for display purposes. we could look into doing something similar for keyof, but that is not guaranteed to work all the time. so not sure it would be a general solution . |
I was suspecting that you'd intern the types so that there is only a single But at least it's reasonably doable... By using a typical equivalence class design you can implement efficient identity checks. Add a There's a trade-off between adding the property to all types (slightly more memory) and a slightly more complex type check using The overall impact on speed should be neglectable. |
if you want to experiment with this idea, i would be happy to help. i would start by |
I have far too many projects at the moment to take on another one, unfortunately. |
I agree this is disappointing, but it looks like it's a hard problem that I hope gets addressed someday! (fingers crossed) The lack of reference finding and refactoring support is such a major limitation that I can't see why anybody uses string literal types for multiple direct assignments in anything but trivial code (unless, of course, they don't need to worry about maintaining it themselves lol). The intellisense certainly looks cool in demos, though! ;-) However, I find the extra magic that advanced index/mapped/etc types add to string literal type usage makes them quite handly for indirectly flowing constraints through definitions in a way that supports references and refactoring in addition to intellisense (especially when combined with things like generic dynamic class generation where string literal types can be maintained). However, it's more more verbose than it would need to be if the language service could somehow figure it all out directly from context and do the right thing. Anyway, it's amazing that we're even able to do any of this stuff at all on top of javascript right now, and I eagerly look foward to each new release :-) |
As asked by @mhegazy in #11929 this issue is for tracking language services for the new
keyof
type.I hope this will surface in the language services like Find references and Rename?
The text was updated successfully, but these errors were encountered: