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

Usage of base class for anonymous class results in erroneous declaration output #42738

Closed
TimvdLippe opened this issue Feb 10, 2021 · 3 comments
Assignees
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@TimvdLippe
Copy link
Contributor

Bug Report

πŸ”Ž Search Terms

mixin declaration

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about v4.2.0-dev.20210207

⏯ Playground Link

Playground link with relevant code

Workbench Repro

πŸ’» Code

// @showEmit
// @showEmittedFile: MyPanel.d.ts

// @declaration
// @filename: singleton.ts
type Constructor<T = {}> = new (...args: any[]) => T;

export function singleton<TBase extends Constructor>(baseClass: TBase) {
  let instance: InstanceType<TBase>;
  return class extends baseClass {
    static instance(opts: {forceNew?: boolean} = {forceNew: undefined}): InstanceType<TBase> {
      const {forceNew} = opts;
      if (!instance || forceNew) {
        instance = new this() as InstanceType<TBase>;
      }

      return instance;
    }
  }
}

// @declaration
// @filename: MyWidget.ts
export class MyWidget {
    public publicMethod(): void {
        this.someMethod();
    }

    public someMethod(): void {

    }
}

// @declaration
// @filename: MyPanel.ts
import {singleton} from './singleton.js';
import {MyWidget} from './MyWidget.js';

// The `.d.ts` file of `MyPanel.ts` includes a new definition of a MyPanel_Base
export class MyPanel extends singleton(class extends MyWidget {}) {}

// A manual declaration of the base class will work as expected
class MySecondPanelBase extends MyWidget {}
export class MySecondPanel extends singleton(MySecondPanelBase) {}

πŸ™ Actual behavior

The .d.ts file for MyPanel includes a full copy of the anonymous class, including its base class. In other words: anything defined in a base class of an anonymous class that is mixed-in gets copied into the .d.ts file. A manual class declaration which gets passed into a mixin declaration does not show the same behavior.

declare const MyPanel_base: {
    new (...args: any[]): {};
    instance(opts?: {
        forceNew?: boolean | undefined;
    }): {
        publicMethod(): void;
        someMethod(): void;
    };
} & {
    new (): {
        publicMethod(): void;
        someMethod(): void;
    };
};
export declare class MyPanel extends MyPanel_base {
}
export {};

πŸ™‚ Expected behavior

Mixing an anonymous class with a base class should generate the same .d.ts with regards to how the base class is mixed-in. In the playground link, that would mean that MyPanel_base does not copy the two methods defined on MyWidget and instead extends MyWidget.

declare class MyPanel_base extends MyWidget {
    new (...args: any[]): {};
    instance(opts?: {
        forceNew?: boolean | undefined;
    }): MyPanel_base;
} & typeof MyPanel_base;
export declare class MyPanel extends MyPanel_base {
}
export {};
@TimvdLippe
Copy link
Contributor Author

For context, the CL where I ran into this problem is https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2687507/1

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Feb 12, 2021
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.3.0 milestone Feb 12, 2021
@weswigham
Copy link
Member

The instance type for your anonymous base class isn't accessible, so it's serialized structurally. class extends MyWidget {} is making a new anonymous subtype of MyWidget whose type is distinct from MyWidget - that anonymous class is then subtyped by the singleton function into another class via the mixin pattern. Since it's being used as an anonymous expression, there's no way to refer directly to (and thus, reuse) the instance type the class expression being passed to singleton makes. We're essentially doing the best we can without either #41587 or the kind of synthetic hoisting and naming described in #44045.

@weswigham weswigham added Design Limitation Constraints of the existing architecture prevent this from being fixed and removed Needs Investigation This issue needs a team member to investigate its status. labels May 24, 2021
@TimvdLippe
Copy link
Contributor Author

#41587 is very interesting and might also solve the singleton decorator type problem (e.g. remove the need for as InstanceType<TBase>;)

Thanks for investigating!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed Rescheduled This issue was previously scheduled to an earlier milestone
Projects
None yet
Development

No branches or pull requests

4 participants