-
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
Improve type inference for circular references in object initializers #45213
Comments
This is not true 🤔 Please follow the issue template, and ensure you include all relevant pieces of code—the definition of |
I've added more info, aaand 😄 found a similar issue #43683 in which you gave an advice about adding a return type annotation. Of course this works in my case: const obj = {
name: 'test',
foo: makeFoo((): number => obj.name.length), // infers type of `obj` correctly :)
} But sadly it doesn't work in this: const obj = {
complex: {
name: 'qwe',
age: 123,
},
bar: makeFoo((): typeof obj.complex => obj.complex)
} So if I have complex types, I need to declare them separately with no auto-infering help from typescript 😟 |
Pretty sure #30134 is your suggestion 😄 |
I don't know, it seems different. Although, example in the last comment looks sligthly similar. |
Yes, I do. |
I've ran into a similar issue, and was able to reproduce it without using generics (so I don't think that #30134 helps to address it?). Here's my code sample: const decorate = (fn: () => boolean): () => boolean => fn
const thing = {
fn: decorate(() => {
type Thing = typeof thing
const thing_: Thing = thing
return 'fn' in thing_
})
} I've tried poking at other threads, and I still fail to understand why this sort of thing is impractical for a type-checker to figure out. The return type of decorate() is known, it's Where's the flaw in my thinking? Why is it not practical/slow for this sort of logic to be done by a type checker? Thanks. |
@theScottyJam The process you're going through there mixes and matches depth-first and breadth-first processing of the program, but you're picking and choosing when you go depth-first vs breadth-first based on facts that you can only ascertain by first reading the entire program and establishing those causal links ahead of time. If you had to "check" a program but could only see one node of code at a time and had to choose what to do next without knowing what the next node you intended to analyze said, you wouldn't be able to do this. It helps if you consider the active call stack of typechecking a program through a depth-first recursive descent of its AST. |
Thanks for the response @RyanCavanaugh However, I'm not sure I fully understand. If TypeScript were forced to parse in such a strict fashion, then code like this would never compile in TypeScript. type A = { x: B }
type B = { y: A } Or this (which is the same as my previous example, but without the // This works
const thing = {
fn: () => {
type Thing = typeof thing
const thing_: Thing = thing
return 'fn' in thing_
}
} I'm not sure how TypeScript handles the above scenarios, but I'm going to try and present a three-pass algorithm that should be able to handle the issue I had presented. (I'm not sure if three passes are strictly necessary to make this work, but it makes explaining it easier). Let's assume that TypeScript analyzes my code in the following order. A. it looks like the In pass 1, all it does is try and figure out what identifiers exist, along with some trivial-to-gather information about them.
The whole purpose of pass 2 is to just try and auto-determin all of the different types. We'll follow a depth-first approach, but we'll go out-of-order if necessary.
Now we know all of the type information, so in phase 3, we can start making sure everything connects together properly, and we aren't doing something silly like |
You're basically describing the process we already have, but one of the steps is in the wrong pass. Specifically, step const decorate = (fn: () => boolean): () => boolean => fn
const thing = {
fn: decorate(() => {
type Thing = typeof thing
const thing_: Thing = thing
return 'fn' in thing_
})
}; You could equivalently have written this program: const decorate = (fn: () => boolean): () => boolean => fn
const x = decorate(() => {
type Thing = typeof thing
const thing_: Thing = thing
return 'fn' in thing_
});
const thing = {
fn: x
} So applying the bind pass to object literals doesn't remove the circularity per se, it just makes it happen in some cases and not others. It's also not generalizable, because you could have written const decorate = (fn: () => boolean): () => boolean => fn
const thing = {
fn: decorate(() => {
type Thing = typeof thing
const thing_: Thing = thing
return 'fn' in thing_
}),
...someExpr
};
const someExpr = thing.fn(); and now the members of const thing = {
fn: {
a: {
return thing.fn.a;
}
}),
...someExpr
}; then the type of Naturally anything is possible; we could rearrange a bunch of things and introduce many new special codepaths to make this work, but in general it doesn't come up that often to justify the additional complexity. Keeping a strict rule that the bind phase only uses declaration constructs and the check phase is the only one that's allowed to use inferred object shapes makes the whole system much easier to reason about, and creates invariants that can be used to improve performance in other cases. |
I don't think there's actually an issue there. The type of So, in this example: const decorate = (fn: () => boolean): () => boolean => fn
const x = decorate(() => {
type Thing = typeof thing
const thing_: Thing = thing
return 'fn' in thing_
});
const thing = {
fn: x
} The following would happen during phase 2:
And here's how the algorithm would be followed with the later code example you shared: const decorate = (fn: () => boolean): () => boolean => fn
const thing = {
fn: decorate(() => {
type Thing = typeof thing
const thing_: Thing = thing
return 'fn' in thing_
}),
...someExpr
};
const someExpr = thing.fn(); (I also gave these steps letters, but in no particular order)
So, I don't think those examples actually break the deterministic logic of the original algorithm I presented, nor does it turn this into a thing that only sometimes works and sometimes doesn't, depending on where you put the code. If it's possible to figure out the type of the thing, it'll figure it out, by running the steps in whatever order necessary, and this should theoretically work as long as you don't have a true circular dependency on types.
Yeah, and that's fair. I don't know anything about the TypeScript codebase, nor do I know how much complexity this would add, but I'm sure it would be a fairly large refactor to make anything like this work. So, I'm fine if such a feature doesn't actually get put in. I mostly wanted to know why this feature didn't exist - was it because (a) some technical limitation with making this algorithm for the general case that I wasn't seeing (which we're now discussing, trying to figure out if there really is such a technical limitation), or (b) it simply hasn't been added yet, and perhaps would never be added, because it's just not useful enough to warrant the amount of work it would take to add it in. |
The mechanism you're describing here where, upon detecting a possible circularity, we just sort of put the relevant question on pause and come back to it later - this isn't really compatible with the straightforward caching recursive descent that the checker is written in. It's instructive to look at the call stack at the point that we log the circularity error and see how every caller in that path (of which the incoming call stack is could be practically any function in the checker) would have to be basically completely rewritten to handle the answer of "ask me again later". We also need to be able to detect circularities instead of looping forever. For example, if you wrote function f1() {
return [0, f2()] as const;
}
function f2() {
return [0, f1()] as const;
} then any sort of work-deferring mechanism needs to be able to detect when it's looping forever so that it doesn't attempt to infer an infinite sequence of The relevant codepaths in the checker here are not especially complicated (relatively speaking) and I'd encourage you to try a PR to understand the problem more deeply. Not to be glib, but if this were low-hanging fruit, we would have done it already 😉. Again, it's not impossible, but it's not at all a straightforward fix - we'd have to rewrite very large portions of the entire checker. |
Yeah, this makes sense. TypeScript uses a recursive descent parsing algorithm, and the sort of algorithm I was describing would require an entirely different type of parsing algorithm. Not only would it take a whole lot of work to switch, it'll also add a lot of complexity to the codebase, and this particular issue isn't an extremely common one. I don't think I'll try to actually make a PR - I can tell that this sort of thing would be a giant refactor, and it's certainly not a task I would be capable of, especially considering that I currently know nothing about the TypeScript codebase. Ok, thanks for helping me understand :) |
When you have something like this (also see a playground):
type of
obj
will be infered asany
, because in order to determine type ofobj
typescript first looks at return type ofmakeFoo(...)
which usesobj
.The actual error is this:
'obj' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.(7022)
What if we could give some hint to a compiler to handle this case? Maybe like this:
Right now I have a workaround for this:
but it would be cool if it could be done more easily.
The text was updated successfully, but these errors were encountered: