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

Recursive definitions in mixins #29872

Open
canonic-epicure opened this issue Feb 12, 2019 · 8 comments
Open

Recursive definitions in mixins #29872

canonic-epicure opened this issue Feb 12, 2019 · 8 comments
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript

Comments

@canonic-epicure
Copy link

Its currently possible to create a circular references in classes just fine:

export class Class1 {
    another     : Class2     // compiles fine
}
export class Class2 {
    another     : Class1    // compiles fine
}

However, the same thing fails when using mixin-based classes:

export const SampleMixin1 = <T extends AnyConstructor<object>>(base : T) =>
class SampleMixin1 extends base {
    another             : SampleMixin2 // TS2502: 'another' is referenced directly or indirectly in its own type annotation
}
export type SampleMixin1 = Mixin<typeof SampleMixin1>


export const SampleMixin2 = <T extends AnyConstructor<object>>(base : T) =>
class SampleMixin2 extends base {
    another             : SampleMixin1 // TS2502: 'another' is referenced directly or indirectly in its own type annotation
}
export type SampleMixin2 = Mixin<typeof SampleMixin2>


// supporting declarations for mixin pattern
export type AnyFunction<A = any>      = (...input: any[]) => A
export type AnyConstructor<A = any>   = new (...input: any[]) => A
export type Mixin<T extends AnyFunction> = InstanceType<ReturnType<T>>

This makes things much more complicated, you need to introduce some dummy interfaces, etc, etc.

The workaround exists - to use different form of creating the type for the standalone mixin class - with interfaces:

export const SampleMixin3 = <T extends AnyConstructor<object>>(base : T) =>
class SampleMixin3 extends base {
    another             : SampleMixin4
}
export interface SampleMixin3 extends Mixin<typeof SampleMixin3> {}


export const SampleMixin4 = <T extends AnyConstructor<object>>(base : T) =>
class SampleMixin4 extends base {
    another             : SampleMixin3
}
export interface SampleMixin4 extends Mixin<typeof SampleMixin4> {}

But this notation seems to drive crazy the IDE (the language server under the hood?). It stop finding usages of properties, show many false type errors etc. Basically in such approach you are limited to old-good console npx tsc launch (which works well at least), but all the development aid facilities don't work.

I think mixin pattern should be supported first class. I'd even create some language construct for it.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Feb 19, 2019
@canonic-epicure
Copy link
Author

Any hope for a proper solution of this issue?

@orta
Copy link
Contributor

orta commented Jun 30, 2020

When using mixins via class expressions this doesn't seem to apply:

type Constructor = new (...args: any[]) => {};

function Scale<TBase extends Constructor>(Base: TBase) {
  return class Scaling extends Base {
    _scale = 1;

    setScale(scale: number) {
      this._scale = scale;
    }

    get scale(): number {
      return this._scale;
    }
  };
}

function Logger<TBase extends Constructor>(Base: TBase) {
  return class Logger extends Base {

  };
}



class Group {
  sprites: typeof EightBitSprite[]
}

class Sprite {
  group: typeof LoggableGroup
}

const EightBitSprite = Scale(Sprite);
const LoggableGroup = Logger(Group);

@canonic-epicure
Copy link
Author

canonic-epicure commented Jun 30, 2020

@orta This is because you haven't created an "abstract" mixin type - a type that represents instance of any class that has consumed a mixin. You only work with specific classes. Abstract LoggerT would look like:

// supporting declarations for mixin pattern
export type AnyFunction<A = any>      = (...input: any[]) => A
export type AnyConstructor<A = any>   = new (...input: any[]) => A
export type Mixin<T extends AnyFunction> = InstanceType<ReturnType<T>>

function Logger<TBase extends Constructor>(Base: TBase) {
  return class Logger extends Base {
 };
}

type LoggerT = Mixin<typeof Logger>  // "abstract" mixin type 

Actually, I just checked and the originally submitted code seems to work starting from 3.8.2!! (The version picker in playground is awesome). It does not work in 3.7.5 and prior.

Hopefully this was an intentional improvement, properly covered with a test, not a random codebase fluctuation. Could you check may be?

@trusktr
Copy link
Contributor

trusktr commented Jul 1, 2020

Yes please, I hope this is tested for. Making robust mixins has been difficult, results have been brittle.

@canonic-epicure
Copy link
Author

Hello again,

As I mentioned in the previous comment, the issue of this ticket seems to be resolved in 3.8.2 release. However, this ticket did not receive any updates at that point, so, I'm suspecting the "fix" might be just a side effect of some other change in the codebase.

Can someone from the dev team please check the originally reported issue and confirm, that this behavior is indeed now "officially" supported (and "pinned" with the test)?

@canonic-epicure
Copy link
Author

No, actually, as I thought, the "fix" was just a random codebase fluctuation. First of all, if you enable strictPropertyInitialization it will fail again with the same message. W/o that option it works:
Playground link

Secondly, it fails for 2nd level of mixins:
Playground link

This is so frustrating. Probably a decent portion of the TS issues is related to the bad support of the mixin pattern. Why not finally implementing a proper solution/workaround? It does not have to be academically pure, it should just work. Can't believe its something impossible.

@trusktr
Copy link
Contributor

trusktr commented Jul 18, 2021

Although the original example still does not work, the errors go away if written like this:

// supporting declarations for mixin pattern
export type AnyFunction<A = any> = (...input: any[]) => A
export type AnyConstructor<T = object, A extends any[] = any[], Static = {}> = (new (...a: A) => T) & Static

export function SampleMixin1<T extends AnyConstructor>(base: T) {
    return class Sample1 extends base {
        another!: InstanceType<ReturnType<typeof SampleMixin2>>
    }
}

export function SampleMixin2<T extends AnyConstructor>(base: T) {
    return class Sample2 extends base {
        another!: InstanceType<ReturnType<typeof SampleMixin1>>
    }
}

However the result is very strange!!!! Sometimes hovering on another will show proper types. Most of the time, it will just show any. There definitely something flaky happening under the hood.

Here is something I've been doing in practice that doesn't have any errors and seems to work (at least in my cases so far):

// supporting declarations for mixin pattern
export type AnyFunction<A = any> = (...input: any[]) => A
export type AnyConstructor<T = object, A extends any[] = any[], Static = {}> = (new (...a: A) => T) & Static

export function SampleMixin1<T extends AnyConstructor>(base: T) {
    return class Sample1 extends base {
        another!: Sample2
    }
}

export const Sample1 = SampleMixin1(class {})
export type Sample1 = typeof Sample1

export function SampleMixin2<T extends AnyConstructor>(base: T) {
    return class Sample2 extends base {
        another!: Sample1
    }
}

export const Sample2 = SampleMixin2(class {})
export type Sample2 = typeof Sample2

@canonic-epicure
Copy link
Author

@trusktr Can you try making it working as in my 2nd example here? Basically, create a SampleMixin3 which extends SampleMixin1 and SampleMixin4, which extends SampleMixin2. Then make a cycle between them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants