-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Allow looking for properties inside the base of a mixin'd class #32770
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
90 changes: 90 additions & 0 deletions
90
tests/baselines/reference/expressionPropertyLookupIncludesMixins.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| //// [expressionPropertyLookupIncludesMixins.ts] | ||
| // https://github.com/microsoft/TypeScript/issues/31426 | ||
|
|
||
| export type AnyFunction<A = any> = (...input : any[]) => A | ||
| export type AnyConstructor<A = object> = new (...input : any[]) => A | ||
| export type Mixin<T extends AnyFunction> = InstanceType<ReturnType<T>> | ||
|
|
||
| export const Box = <T extends AnyConstructor<object>>(base : T) => | ||
| class Box extends base { | ||
| value : any | ||
| } | ||
| export interface Box extends Mixin<typeof Box> {} | ||
|
|
||
| export const Observable = <T extends AnyConstructor<object>>(base : T) => | ||
| class Observable extends base { | ||
| observe () : IQuark { | ||
| return | ||
| } | ||
| } | ||
| export interface Observable extends Mixin<typeof Observable> {} | ||
|
|
||
| export const CQuark = <T extends AnyConstructor<Box & Observable>>(base : T) => | ||
| class Quark extends base { | ||
|
|
||
| observe () : Quark { | ||
| // No error here! | ||
| this.value | ||
|
|
||
|
|
||
| return | ||
| } | ||
| } | ||
| export interface IQuark extends Mixin<typeof CQuark> {} | ||
|
|
||
| const test = (a : IQuark) => a.value // <-- Should not error | ||
|
|
||
|
|
||
| //// [expressionPropertyLookupIncludesMixins.js] | ||
| "use strict"; | ||
| // https://github.com/microsoft/TypeScript/issues/31426 | ||
| var __extends = (this && this.__extends) || (function () { | ||
| var extendStatics = function (d, b) { | ||
| extendStatics = Object.setPrototypeOf || | ||
| ({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) || | ||
| function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; }; | ||
| return extendStatics(d, b); | ||
| }; | ||
| return function (d, b) { | ||
| extendStatics(d, b); | ||
| function __() { this.constructor = d; } | ||
| d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __()); | ||
| }; | ||
| })(); | ||
| exports.__esModule = true; | ||
| exports.Box = function (base) { | ||
| return /** @class */ (function (_super) { | ||
| __extends(Box, _super); | ||
| function Box() { | ||
| return _super !== null && _super.apply(this, arguments) || this; | ||
| } | ||
| return Box; | ||
| }(base)); | ||
| }; | ||
| exports.Observable = function (base) { | ||
| return /** @class */ (function (_super) { | ||
| __extends(Observable, _super); | ||
| function Observable() { | ||
| return _super !== null && _super.apply(this, arguments) || this; | ||
| } | ||
| Observable.prototype.observe = function () { | ||
| return; | ||
| }; | ||
| return Observable; | ||
| }(base)); | ||
| }; | ||
| exports.CQuark = function (base) { | ||
| return /** @class */ (function (_super) { | ||
| __extends(Quark, _super); | ||
| function Quark() { | ||
| return _super !== null && _super.apply(this, arguments) || this; | ||
| } | ||
| Quark.prototype.observe = function () { | ||
| // No error here! | ||
| this.value; | ||
| return; | ||
| }; | ||
| return Quark; | ||
| }(base)); | ||
| }; | ||
| var test = function (a) { return a.value; }; // <-- Should not error |
105 changes: 105 additions & 0 deletions
105
tests/baselines/reference/expressionPropertyLookupIncludesMixins.symbols
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| === tests/cases/compiler/expressionPropertyLookupIncludesMixins.ts === | ||
| // https://github.com/microsoft/TypeScript/issues/31426 | ||
|
|
||
| export type AnyFunction<A = any> = (...input : any[]) => A | ||
| >AnyFunction : Symbol(AnyFunction, Decl(expressionPropertyLookupIncludesMixins.ts, 0, 0)) | ||
| >A : Symbol(A, Decl(expressionPropertyLookupIncludesMixins.ts, 2, 24)) | ||
| >input : Symbol(input, Decl(expressionPropertyLookupIncludesMixins.ts, 2, 43)) | ||
| >A : Symbol(A, Decl(expressionPropertyLookupIncludesMixins.ts, 2, 24)) | ||
|
|
||
| export type AnyConstructor<A = object> = new (...input : any[]) => A | ||
| >AnyConstructor : Symbol(AnyConstructor, Decl(expressionPropertyLookupIncludesMixins.ts, 2, 65)) | ||
| >A : Symbol(A, Decl(expressionPropertyLookupIncludesMixins.ts, 3, 27)) | ||
| >input : Symbol(input, Decl(expressionPropertyLookupIncludesMixins.ts, 3, 47)) | ||
| >A : Symbol(A, Decl(expressionPropertyLookupIncludesMixins.ts, 3, 27)) | ||
|
|
||
| export type Mixin<T extends AnyFunction> = InstanceType<ReturnType<T>> | ||
| >Mixin : Symbol(Mixin, Decl(expressionPropertyLookupIncludesMixins.ts, 3, 69)) | ||
| >T : Symbol(T, Decl(expressionPropertyLookupIncludesMixins.ts, 4, 18)) | ||
| >AnyFunction : Symbol(AnyFunction, Decl(expressionPropertyLookupIncludesMixins.ts, 0, 0)) | ||
| >InstanceType : Symbol(InstanceType, Decl(lib.es5.d.ts, --, --)) | ||
| >ReturnType : Symbol(ReturnType, Decl(lib.es5.d.ts, --, --)) | ||
| >T : Symbol(T, Decl(expressionPropertyLookupIncludesMixins.ts, 4, 18)) | ||
|
|
||
| export const Box = <T extends AnyConstructor<object>>(base : T) => | ||
| >Box : Symbol(Box, Decl(expressionPropertyLookupIncludesMixins.ts, 6, 12), Decl(expressionPropertyLookupIncludesMixins.ts, 9, 1)) | ||
| >T : Symbol(T, Decl(expressionPropertyLookupIncludesMixins.ts, 6, 20)) | ||
| >AnyConstructor : Symbol(AnyConstructor, Decl(expressionPropertyLookupIncludesMixins.ts, 2, 65)) | ||
| >base : Symbol(base, Decl(expressionPropertyLookupIncludesMixins.ts, 6, 54)) | ||
| >T : Symbol(T, Decl(expressionPropertyLookupIncludesMixins.ts, 6, 20)) | ||
|
|
||
| class Box extends base { | ||
| >Box : Symbol(Box, Decl(expressionPropertyLookupIncludesMixins.ts, 6, 66)) | ||
| >base : Symbol(base, Decl(expressionPropertyLookupIncludesMixins.ts, 6, 54)) | ||
|
|
||
| value : any | ||
| >value : Symbol(Box.value, Decl(expressionPropertyLookupIncludesMixins.ts, 7, 24)) | ||
| } | ||
| export interface Box extends Mixin<typeof Box> {} | ||
| >Box : Symbol(Box, Decl(expressionPropertyLookupIncludesMixins.ts, 6, 12), Decl(expressionPropertyLookupIncludesMixins.ts, 9, 1)) | ||
| >Mixin : Symbol(Mixin, Decl(expressionPropertyLookupIncludesMixins.ts, 3, 69)) | ||
| >Box : Symbol(Box, Decl(expressionPropertyLookupIncludesMixins.ts, 6, 12), Decl(expressionPropertyLookupIncludesMixins.ts, 9, 1)) | ||
|
|
||
| export const Observable = <T extends AnyConstructor<object>>(base : T) => | ||
| >Observable : Symbol(Observable, Decl(expressionPropertyLookupIncludesMixins.ts, 12, 12), Decl(expressionPropertyLookupIncludesMixins.ts, 17, 1)) | ||
| >T : Symbol(T, Decl(expressionPropertyLookupIncludesMixins.ts, 12, 27)) | ||
| >AnyConstructor : Symbol(AnyConstructor, Decl(expressionPropertyLookupIncludesMixins.ts, 2, 65)) | ||
| >base : Symbol(base, Decl(expressionPropertyLookupIncludesMixins.ts, 12, 61)) | ||
| >T : Symbol(T, Decl(expressionPropertyLookupIncludesMixins.ts, 12, 27)) | ||
|
|
||
| class Observable extends base { | ||
| >Observable : Symbol(Observable, Decl(expressionPropertyLookupIncludesMixins.ts, 12, 73)) | ||
| >base : Symbol(base, Decl(expressionPropertyLookupIncludesMixins.ts, 12, 61)) | ||
|
|
||
| observe () : IQuark { | ||
| >observe : Symbol(Observable.observe, Decl(expressionPropertyLookupIncludesMixins.ts, 13, 31)) | ||
| >IQuark : Symbol(IQuark, Decl(expressionPropertyLookupIncludesMixins.ts, 30, 1)) | ||
|
|
||
| return | ||
| } | ||
| } | ||
| export interface Observable extends Mixin<typeof Observable> {} | ||
| >Observable : Symbol(Observable, Decl(expressionPropertyLookupIncludesMixins.ts, 12, 12), Decl(expressionPropertyLookupIncludesMixins.ts, 17, 1)) | ||
| >Mixin : Symbol(Mixin, Decl(expressionPropertyLookupIncludesMixins.ts, 3, 69)) | ||
| >Observable : Symbol(Observable, Decl(expressionPropertyLookupIncludesMixins.ts, 12, 12), Decl(expressionPropertyLookupIncludesMixins.ts, 17, 1)) | ||
|
|
||
| export const CQuark = <T extends AnyConstructor<Box & Observable>>(base : T) => | ||
| >CQuark : Symbol(CQuark, Decl(expressionPropertyLookupIncludesMixins.ts, 20, 12)) | ||
| >T : Symbol(T, Decl(expressionPropertyLookupIncludesMixins.ts, 20, 23)) | ||
| >AnyConstructor : Symbol(AnyConstructor, Decl(expressionPropertyLookupIncludesMixins.ts, 2, 65)) | ||
| >Box : Symbol(Box, Decl(expressionPropertyLookupIncludesMixins.ts, 6, 12), Decl(expressionPropertyLookupIncludesMixins.ts, 9, 1)) | ||
| >Observable : Symbol(Observable, Decl(expressionPropertyLookupIncludesMixins.ts, 12, 12), Decl(expressionPropertyLookupIncludesMixins.ts, 17, 1)) | ||
| >base : Symbol(base, Decl(expressionPropertyLookupIncludesMixins.ts, 20, 67)) | ||
| >T : Symbol(T, Decl(expressionPropertyLookupIncludesMixins.ts, 20, 23)) | ||
|
|
||
| class Quark extends base { | ||
| >Quark : Symbol(Quark, Decl(expressionPropertyLookupIncludesMixins.ts, 20, 79)) | ||
| >base : Symbol(base, Decl(expressionPropertyLookupIncludesMixins.ts, 20, 67)) | ||
|
|
||
| observe () : Quark { | ||
| >observe : Symbol(Quark.observe, Decl(expressionPropertyLookupIncludesMixins.ts, 21, 26)) | ||
| >Quark : Symbol(Quark, Decl(expressionPropertyLookupIncludesMixins.ts, 20, 79)) | ||
|
|
||
| // No error here! | ||
| this.value | ||
| >this.value : Symbol(Box.value, Decl(expressionPropertyLookupIncludesMixins.ts, 7, 24)) | ||
| >this : Symbol(Quark, Decl(expressionPropertyLookupIncludesMixins.ts, 20, 79)) | ||
| >value : Symbol(Box.value, Decl(expressionPropertyLookupIncludesMixins.ts, 7, 24)) | ||
|
|
||
|
|
||
| return | ||
| } | ||
| } | ||
| export interface IQuark extends Mixin<typeof CQuark> {} | ||
| >IQuark : Symbol(IQuark, Decl(expressionPropertyLookupIncludesMixins.ts, 30, 1)) | ||
| >Mixin : Symbol(Mixin, Decl(expressionPropertyLookupIncludesMixins.ts, 3, 69)) | ||
| >CQuark : Symbol(CQuark, Decl(expressionPropertyLookupIncludesMixins.ts, 20, 12)) | ||
|
|
||
| const test = (a : IQuark) => a.value // <-- Should not error | ||
| >test : Symbol(test, Decl(expressionPropertyLookupIncludesMixins.ts, 33, 5)) | ||
| >a : Symbol(a, Decl(expressionPropertyLookupIncludesMixins.ts, 33, 14)) | ||
| >IQuark : Symbol(IQuark, Decl(expressionPropertyLookupIncludesMixins.ts, 30, 1)) | ||
| >a.value : Symbol(Box.value, Decl(expressionPropertyLookupIncludesMixins.ts, 7, 24)) | ||
| >a : Symbol(a, Decl(expressionPropertyLookupIncludesMixins.ts, 33, 14)) | ||
| >value : Symbol(Box.value, Decl(expressionPropertyLookupIncludesMixins.ts, 7, 24)) | ||
|
|
83 changes: 83 additions & 0 deletions
83
tests/baselines/reference/expressionPropertyLookupIncludesMixins.types
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| === tests/cases/compiler/expressionPropertyLookupIncludesMixins.ts === | ||
| // https://github.com/microsoft/TypeScript/issues/31426 | ||
|
|
||
| export type AnyFunction<A = any> = (...input : any[]) => A | ||
| >AnyFunction : AnyFunction<A> | ||
| >input : any[] | ||
|
|
||
| export type AnyConstructor<A = object> = new (...input : any[]) => A | ||
| >AnyConstructor : AnyConstructor<A> | ||
| >input : any[] | ||
|
|
||
| export type Mixin<T extends AnyFunction> = InstanceType<ReturnType<T>> | ||
| >Mixin : InstanceType<ReturnType<T>> | ||
|
|
||
| export const Box = <T extends AnyConstructor<object>>(base : T) => | ||
| >Box : <T extends AnyConstructor<object>>(base: T) => { new (...input: any[]): Box; prototype: Box<any>.Box; } & T | ||
| ><T extends AnyConstructor<object>>(base : T) =>class Box extends base { value : any} : <T extends AnyConstructor<object>>(base: T) => { new (...input: any[]): Box; prototype: Box<any>.Box; } & T | ||
| >base : T | ||
|
|
||
| class Box extends base { | ||
| >class Box extends base { value : any} : { new (...input: any[]): Box; prototype: Box<any>.Box; } & T | ||
| >Box : { new (...input: any[]): Box; prototype: Box<any>.Box; } & T | ||
| >base : object | ||
|
|
||
| value : any | ||
| >value : any | ||
| } | ||
| export interface Box extends Mixin<typeof Box> {} | ||
| >Box : <T extends AnyConstructor<object>>(base: T) => { new (...input: any[]): Box; prototype: Box<any>.Box; } & T | ||
|
|
||
| export const Observable = <T extends AnyConstructor<object>>(base : T) => | ||
| >Observable : <T extends AnyConstructor<object>>(base: T) => { new (...input: any[]): Observable; prototype: Observable<any>.Observable; } & T | ||
| ><T extends AnyConstructor<object>>(base : T) =>class Observable extends base { observe () : IQuark { return }} : <T extends AnyConstructor<object>>(base: T) => { new (...input: any[]): Observable; prototype: Observable<any>.Observable; } & T | ||
| >base : T | ||
|
|
||
| class Observable extends base { | ||
| >class Observable extends base { observe () : IQuark { return }} : { new (...input: any[]): Observable; prototype: Observable<any>.Observable; } & T | ||
| >Observable : { new (...input: any[]): Observable; prototype: Observable<any>.Observable; } & T | ||
| >base : object | ||
|
|
||
| observe () : IQuark { | ||
| >observe : () => IQuark | ||
|
|
||
| return | ||
| } | ||
| } | ||
| export interface Observable extends Mixin<typeof Observable> {} | ||
| >Observable : <T extends AnyConstructor<object>>(base: T) => { new (...input: any[]): Observable; prototype: Observable<any>.Observable; } & T | ||
|
|
||
| export const CQuark = <T extends AnyConstructor<Box & Observable>>(base : T) => | ||
| >CQuark : <T extends AnyConstructor<Box & Observable>>(base: T) => { new (...input: any[]): Quark; prototype: CQuark<any>.Quark; } & T | ||
| ><T extends AnyConstructor<Box & Observable>>(base : T) =>class Quark extends base { observe () : Quark { // No error here! this.value return }} : <T extends AnyConstructor<Box & Observable>>(base: T) => { new (...input: any[]): Quark; prototype: CQuark<any>.Quark; } & T | ||
| >base : T | ||
|
|
||
| class Quark extends base { | ||
| >class Quark extends base { observe () : Quark { // No error here! this.value return }} : { new (...input: any[]): Quark; prototype: CQuark<any>.Quark; } & T | ||
| >Quark : { new (...input: any[]): Quark; prototype: CQuark<any>.Quark; } & T | ||
| >base : Box & Observable | ||
|
|
||
| observe () : Quark { | ||
| >observe : () => Quark | ||
|
|
||
| // No error here! | ||
| this.value | ||
| >this.value : any | ||
| >this : this | ||
| >value : any | ||
|
|
||
|
|
||
| return | ||
| } | ||
| } | ||
| export interface IQuark extends Mixin<typeof CQuark> {} | ||
| >CQuark : <T extends AnyConstructor<Box & Observable>>(base: T) => { new (...input: any[]): Quark; prototype: CQuark<any>.Quark; } & T | ||
|
|
||
| const test = (a : IQuark) => a.value // <-- Should not error | ||
| >test : (a: IQuark) => any | ||
| >(a : IQuark) => a.value : (a: IQuark) => any | ||
| >a : IQuark | ||
| >a.value : any | ||
| >a : IQuark | ||
| >value : any | ||
|
|
34 changes: 34 additions & 0 deletions
34
tests/cases/compiler/expressionPropertyLookupIncludesMixins.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| // https://github.com/microsoft/TypeScript/issues/31426 | ||
|
|
||
| export type AnyFunction<A = any> = (...input : any[]) => A | ||
| export type AnyConstructor<A = object> = new (...input : any[]) => A | ||
| export type Mixin<T extends AnyFunction> = InstanceType<ReturnType<T>> | ||
|
|
||
| export const Box = <T extends AnyConstructor<object>>(base : T) => | ||
| class Box extends base { | ||
| value : any | ||
| } | ||
| export interface Box extends Mixin<typeof Box> {} | ||
|
|
||
| export const Observable = <T extends AnyConstructor<object>>(base : T) => | ||
| class Observable extends base { | ||
| observe () : IQuark { | ||
| return | ||
| } | ||
| } | ||
| export interface Observable extends Mixin<typeof Observable> {} | ||
|
|
||
| export const CQuark = <T extends AnyConstructor<Box & Observable>>(base : T) => | ||
| class Quark extends base { | ||
|
|
||
| observe () : Quark { | ||
| // No error here! | ||
| this.value | ||
|
|
||
|
|
||
| return | ||
| } | ||
| } | ||
| export interface IQuark extends Mixin<typeof CQuark> {} | ||
|
|
||
| const test = (a : IQuark) => a.value // <-- Should not error |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Properties are supposed to get merged down into the types that inherit them - check out
resolveObjectTypeMembers. You should never need to traverse "base types" to find a property like this, as base types are a syntactic construct only (we're a structural typesystem, after all). I would check out why the property isn't being included there, instead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a look at this, and the issue is that there’s a circularity resolving the base types
IQuarkwhile checking its declaration. As soon as we start resolving base types, we settype.resolvedBaseTypesto an empty array, which prevents actually crashing due to this circularity. During that resolution, something needs the resolved members ofIQuark, which sees no base types because they’re still resolving, so it has itsmembersset to an empty array.Later, when we check
a.valueagainstIQuark, its base types have been resolved correctly, but because we’ve cached an emptymembersmap, we don’t try to re-resolve them, so they appear empty.The circularity that occurred was really deep (happened during the instantiation of the conditional
InstanceType) and I didn’t put tons of effort into understanding exactly why it happened. It does seem like you should be able to substituteAininterface A extends B<T> {}forB<T>, so it seems like the best way forward is to see if the circularity can be eliminated. I was able to fix the error by clearingtype.membersif it changed while resolving base types since that seems like a good indicator that something went awry, but that feels pretty hacky.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, hang on—
observe()returns anIQuark, so the circularity is real and clear! It’s even called out explicitly in #29872:Yep, because there’s a circularity that we falsely resolve and don’t report. I think the bug is that we don’t report a circularity error— the “workaround” is the bug.
And my earlier comment:
It turns out the simple form
B<T>(Mixin<typeof CQuark>in this test case) simply resolves toanyin this case without triggering any errors, so that doesn’t actually work either, so I feel better about this interface example being an error. (I guess in the type reference case, we detect the circularity but fail to emit a noImplicitAny error on the way back out.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should report circularity errors when resolving base types, but I would push this to 3.8 to avoid a last-minute breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I'd consider this one a bug and 29872 could be about supporting the actual feature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#33050 originally fixed your case but it was scoped down due to performance reasons. You can see what I mean in the playground link from my comment above.
SampleMixin1andSampleMixin2don’t qualify for deferred resolution of structure because they’re not tuple or array types. If you make them tuple or array types, your example no longer errors, but you’re stuck with tuple/array types where you didn’t want them.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see.
Yup, how is the trick with tuples different from using the
interface? I tried to continue your example, but indeed stuck when trying to access the nested property: linkPerhaps its possible to add some additional type, that will resolve the tuple type.. Something like this - which currently fails.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s not possible. The trick with tuples/arrays is unique to tuples/arrays because they’re explicitly allowed to be recursive via #33050. If you pull the element out of the tuple/array, it’s no longer a tuple/array; it’s an indexed access type and it no longer falls into the special case set up by #33050. As far as I can tell, the only way to get what you’re really asking for is to expand the cases where recursive type aliases are allowed to include generic instantiations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andrewbranch Is this something that #33050 provides? If so, can it be enabled by some flag? #33050 says its not enabled globally, because of the 5% performance hit. I believe for many people, the value of the circular types is much more than those 5%.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it is not.