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

TypeScript 4.0 causes unsolvable TS2611 error for properties inherited from a mixin #41347

Closed
octogonz opened this issue Oct 31, 2020 · 10 comments · Fixed by #41994
Closed

TypeScript 4.0 causes unsolvable TS2611 error for properties inherited from a mixin #41347

octogonz opened this issue Oct 31, 2020 · 10 comments · Fixed by #41994
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@octogonz
Copy link

octogonz commented Oct 31, 2020

TypeScript Version: 4.0.5

Search Terms: TS2611 mixin

Code

// Mixin utilities
export type Constructor<T = {}> = new (...args: any[]) => T;
export type PropertiesOf<T> = { [K in keyof T]: T[K] };

interface IApiItemConstructor extends Constructor<ApiItem>, PropertiesOf<typeof ApiItem> {}

// Base class
class ApiItem {
  public get members(): ReadonlyArray<ApiItem> {
    return [];
  }  
}

// Normal subclass
class ApiEnumMember extends ApiItem {
}

// Mixin base class
interface ApiItemContainerMixin extends ApiItem {
  readonly members: ReadonlyArray<ApiItem>;
}

function ApiItemContainerMixin<TBaseClass extends IApiItemConstructor>(
  baseClass: TBaseClass  
): TBaseClass & (new (...args: any[]) => ApiItemContainerMixin) {
  abstract class MixedClass extends baseClass implements ApiItemContainerMixin {
    public constructor(...args: any[]) {
      super(...args);
    }

    public get members(): ReadonlyArray<ApiItem> {
      return [];
    }
  }

  return MixedClass;
}

// Subclass inheriting from mixin
export class ApiEnum extends ApiItemContainerMixin(ApiItem) {
  // This worked prior to TypeScript 4.0:
  public get members(): ReadonlyArray<ApiEnumMember> {
    return [];
  }  
}

This is a simplification of a real-world problem encountered with API Extractor, which can be reproed as follows:

  1. Clone the branch https://github.com/microsoft/rushstack/tree/octogonz/ts-37894
  2. rush install
  3. rush build --to api-documenter

Expected behavior:

Using TypeScript 3.x, the above code compiled without errors.

Actual behavior:

Using TypeScript 4.0, we get this error:

'members' is defined as a property in class 'ApiItem & ApiItemContainerMixin', but is overridden here in 'ApiEnum' as an accessor. (TS2611)
  • I was not able to workaround the problem by adding // @ts-ignore to the members property, because // @ts-ignore does not get emitted in the .d.ts file. Thus the error will reappear when a consumer of this API tries to compile the .d.ts file.
  • The consumer of the API cannot add // @ts-ignore, because they cannot edit the .d.ts file.
  • This is a case where a project compiled using TypeScript 3.x cannot be consumed by a project that compiled using TypeScript 4.x.
  • The seemingly "correct" solution would be to declare the interface member as a getter instead of using readonly, but that is not an allowable syntax.

The solution that worked was to delete the ApiItemContainerMixin.members property declaration. This was possible in my case only because the base class ApiItem.member happened to also declare that property. It would not be generally true.

Playground Link: Link

Related Issues: This is a regression introduced by PR #37894. In the PR comments, @sandersn asked me to open a separate issue to track this.

@sandersn sandersn added this to the TypeScript 4.2.0 milestone Nov 2, 2020
@sandersn sandersn self-assigned this Nov 2, 2020
@sandersn sandersn added the Bug A bug in TypeScript label Nov 2, 2020
@sandersn
Copy link
Member

sandersn commented Nov 2, 2020

Probably the check needs special handling for synthetic properties, and should be disabled when any source property is legal.

@ricardotestuser
Copy link

up

@sandersn sandersn added this to Not started in Rolling Work Tracking Jan 20, 2021
@sandersn sandersn moved this from Not started to In Progress in Rolling Work Tracking Feb 3, 2021
sandersn added a commit that referenced this issue Feb 4, 2021
This is the simplest fix -- I'm going to check whether it's worthwhile
to make it stricter next.

Fixes #41347
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Feb 4, 2021
@RyanCavanaugh RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label Mar 4, 2021
@zoran995
Copy link

zoran995 commented Aug 9, 2021

@sandersn for #41994 we are using something like this in 3.9.7 (playground link)

// Mixin utilities
type Complete<T> = {
  [P in keyof Required<T>]: Pick<T, P> extends Required<Pick<T, P>>
    ? T[P]
    : T[P] | undefined;
};
export type Constructor<T = {}> = new (...args: any[]) => T;
export type PropertiesOf<T> = { [K in keyof T]: T[K] };

class ModelBase {}
interface ModelInterface<T extends ModelBase> {}
type ModelType<T> = ModelInterface<T> & PropertiesOf<Complete<T>>;

export default function CreateModel<T extends Constructor<ModelBase>>(
  Traits: T
): Constructor<ModelType<InstanceType<T>>> {
  abstract class Model extends ModelBase {}
  return Model as any;
}

// Base class
class ApiItem extends ModelBase {
  readonly test: string = "test";
  public get members(): ReadonlyArray<ApiItem> {
    return [];
  }
}

function ApiItemContainerMixin<
  TBaseClass extends Constructor<ModelType<ApiItem>>
>(baseClass: TBaseClass) {
  abstract class MixedClass extends baseClass {
    public constructor(...args: any[]) {
      super(...args);
      this.test;
    }

    public get members(): ReadonlyArray<ApiItem> { // this fails with ts2611
      return [];
    }
  }

  return MixedClass;
}

// Normal subclass
class ApiEnumMember extends ApiItem {}

// Subclass inheriting from mixin
export class ApiEnum extends ApiItemContainerMixin(CreateModel(ApiItem)) {
  constructor() {
    super();
    this.test;
  }
  // This worked prior to TypeScript 4.0:
  public get members(): ReadonlyArray<ApiEnumMember> { // this fails with ts2611
    return [];
  }
}

I just runned tests using dev build of #41994 and it still fails with

'members' is defined as a property in class 'ModelType<ApiItem>', but is overridden here in 'MixedClass' as an accessor.ts(2611)

and

'members' is defined as a property in class 'ApiItemContainerMixin<Constructor<ModelType<ApiItem>>>.MixedClass & ModelInterface<ApiItem> & PropertiesOf<...>', but is overridden here in 'ApiEnum' as an accessor.ts(2611)

Code is simplified version from Terriajs (v8), it still uses typescript 3.9.7 since this is used through entire codebase. Hopefully I didn't miss anything

@rskopal
Copy link

rskopal commented Sep 14, 2021

The TS2611 error also occurs when I use project references and the mixin function is defined in one of the referenced projects. TypeScript does not generate accessors for properties of the anonymous mixin class in .d.ts file. If I try to override some of these accessors in another project, I get TS2611 error "someprop is defined as a property in class but is overridden here in as an accessor". Minimal demo repo is here.

//Project Mixins, module EventsSubscriber.ts
type Constructor<T = {}> = new (...args: any[]) => T;

export function EventsSubscriber<T extends Constructor<{}>>(Base: T) {
  return class extends Base {
    constructor(...args: any[]) {
      super(...args);
    }

   //eventsFilter getter can be overriden in derived classes
   get eventsFilter(): string[] {
      return [];
    }

    handleEvent(event: string) {
      if (this.eventsFilter.includes(event)) {
        console.log(`Event ${event} was handled.`);
      }
    }
  }
}

//Generated EventsSubscriber.d.ts:
declare type Constructor<T = {}> = new (...args: any[]) => T;
declare function EventsSubscriber<T extends Constructor<{}>>(Base: T): {
    new (...args: any[]): {
        readonly eventsFilter: string[];
        handleEvent(event: string): void;
    };
} & T;


//Project App references project Mixins, module App.ts:
import { EventsSubscriber } from '../mixins/src/EventsSubscriber'

class MyStoreBase {
}

class MyStoreWithEvents extends EventsSubscriber(MyStoreBase) {
  //error TS2611: 'eventsFilter' is defined as a property in class '{ readonly eventsFilter: string[]; handleEvent(event: string): void; } & MyStoreBase',
  //but is overridden here in 'MyStoreWithEvents' as an accessor.
  get eventsFilter(): string[] { //
      return ['event1', 'event2']
  }
}

@frank-weindel
Copy link

I'm also having the same exact issue @rskopal is having regarding mixins.

Rolling Work Tracking automation moved this from Fix available to Done May 31, 2022
@angelaki
Copy link

Should this issue have been fixed? In a pretty complex constellation I'm facing it again, anyone interested in a minimal repro? So I could try to isolate it.

@sandersn
Copy link
Member

@angelaki Please open a new issue with your repro.

@TechQuery
Copy link

@sandersn Has your code in PR #41994 been released to latest TypeScript?

@angelaki
Copy link

@angelaki Please open a new issue with your repro.

Means it would be interesting? Actually it's super weird and I fear quite hard to build a minimal repro. I cannot share the whole repo since it's closed source.

Due to Angular I'm currently on TS 4.7.4.

@angelaki
Copy link

Sorry but I just can't get it reproduced. The weirdest thing is: it compiles and starts. After doing any change in any file and saving it, the rebuild (using Angular) says:

Error: src/base/model.component.base.ts:61:9 - error TS2611: 'model' is defined as a property in class 'mixinTest<Partial<Model1 & Model2 & Model3 & Model4 & ... 5 more ... & { ...; }>[]>.(Anonymous function)<...>.MixinTest & ModelArrayComponentDefault<...>', but is overridden here in 'TestComponent' as an accessor.

What just isn't true. The only place where it is defined is in model.component.base.ts:

ModelComponentBase<T> {
   get model(): T { return this.model$.value; }
}

This is super annoying! Any idea where I could start looking for this issue?

Just updated to TS 4.8.3 but still same behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Rescheduled This issue was previously scheduled to an earlier milestone
Projects