Skip to content

#46218 breaks ramda types on DT #46582

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
sandersn opened this issue Oct 29, 2021 · 10 comments · Fixed by #46586
Closed

#46218 breaks ramda types on DT #46582

sandersn opened this issue Oct 29, 2021 · 10 comments · Fixed by #46586
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Recent Regression This is a new regression just found in the last major/minor version of TypeScript.

Comments

@sandersn
Copy link
Member

sandersn commented Oct 29, 2021

Edit from DanielRosenwasser: #46218 #46218 #46218 #46218 #46218 #46218

I'm working on a standalone repro, but you can see the error now by opening types/ramda/tools.d.ts and looking at lines 149 and 179:

export type Evolvable<E extends Evolver> = {
    [P in keyof E]?: Evolved<E[P]>;
};
// ......
export type Evolver<T extends Evolvable<any> = any> = {
    // if T[K] isn't evolvable, don't allow nesting for that property
    [key in keyof Partial<T>]: ((value: T[key]) => T[key]) | (T[key] extends Evolvable<any> ? Evolver<T[key]> : never);
};

There are now errors on constraints for E and T saying that "Type parameter 'E' has a circular constraint.".

@sandersn sandersn added the Bug A bug in TypeScript label Oct 29, 2021
@sandersn sandersn added this to the TypeScript 4.5.1 milestone Oct 29, 2021
@sandersn
Copy link
Member Author

Uh, here it is:

export type Evolvable<E extends Evolver> = {
    [P in keyof E]: never;
};
export type Evolver<T extends Evolvable<any> = any> = {
    [key in keyof Partial<T>]: never;
};

That looks circular to me. But 4.4 doesn't complain about it.

@sandersn sandersn added the Recent Regression This is a new regression just found in the last major/minor version of TypeScript. label Oct 29, 2021
@andrewbranch
Copy link
Member

I wonder if when we’re trying to see if the base constraint is an array, we can just say “no” without issuing a circularity error if we hit a circularity.

@ahejlsberg
Copy link
Member

The issue is that when we discover a circularity, we record that fact (and issue an error), and the type in question will then appear to have no constraint. Basically, we need to check if attempting to resolve the constraint would cause a circularity and, if so, not obtain the constraint. But we're not equipped to do such a check--at least not without revising a number of code paths.

@andrewbranch
Copy link
Member

Why can't we just add a parameter to getResolvedBaseConstraint that, upon discovering the circularity, bails instead of erroring and caching?

@ahejlsberg
Copy link
Member

Because you'd have to thread that through a whole bunch of different code paths so that you don't cache anything for any of the other resolutions that were part of forming the circle.

@andrewbranch
Copy link
Member

Ah, I missed that almost everything that computeBaseConstraint calls does its own caching 😕

@ahejlsberg
Copy link
Member

Yup. However, I think I have a simpler solution. We can use a check of the form

findResolutionCycleStartIndex(typeVariable, TypeSystemPropertyName.ImmediateBaseConstraint)

to detect if we're already in the process of obtaining the constraint of the type variable, and, if so, skip the array check.

@DanielRosenwasser
Copy link
Member

The alternative to my PR is just to have another overload on Promise.all that takes any. Overload resolution goes through an initial pass using the subtype relationship so that an any doesn't succeed with the first overload.

The question is whether we could always guarantee that the any overload goes last - I don't think we try to guarantee any ordering of lib.d.ts.

@andrewbranch
Copy link
Member

andrewbranch commented Oct 29, 2021

Also possible workaround for the type definitions:

+ interface AnyEvolver extends Evolver<any> {}

/**
 * Represents all objects evolvable with Evolver E
 * @param E
 */
+ export type Evolvable<E extends AnyEvolver> = {
- export type Evolvable<E extends Evolver> = {
    [P in keyof E]?: Evolved<E[P]>;
};

@ahejlsberg
Copy link
Member

The solution I proposed above appears to fix it. Just running tests now to see that it doesn't affect anything else.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Recent Regression This is a new regression just found in the last major/minor version of TypeScript.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants