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

declarations file of extended class type is needlessly verbose (excessively deep) #54533

Open
clshortfuse opened this issue Jun 5, 2023 · 1 comment
Assignees
Labels
Needs Investigation This issue needs a team member to investigate its status. Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@clshortfuse
Copy link

clshortfuse commented Jun 5, 2023

Bug Report

When returning an extended class type from a function, the resulting .d.ts file is needlessly verbose resulting in very, very long .d.ts for chained extensions. (See playground for details).

🔎 Search Terms

emitDeclarations
polymorphic this
mixins
Base constructor must have same return type
Type instantiation is excessively deep and possibly infinite

🕗 Version & Regression Information

  • Version v4.2.3 is the minimum version where the code can run without errors.
  • All versions output this repeated structure

⏯ Playground Link

https://www.typescriptlang.org/play?target=7&ts=5.2.0-dev.20230605#code/MYGwhgzhAEDCCuEAuB7AtgURAUzdgdktNgB5IEAmMAEgCoCyAMlrgUQN4CwAUNNAPQAqQdB59oIgKoRsOKNGAp8yAE7xgqFdHzZsFPdFTQA7ihUBrMCpTx8FaLQDKYviIAGAIUjYFS1es0YNEQiMBAQaAALMAA3HyRInwgwPGgVbCR4FXxDAE8AB2wAOjcXCX4XRWUkNQ0zAAoipqsAcwgALmgwfFyASmguXnEIeEKVet6AbhcAXx4XIWgMMkoYWHAoIvzrVCQCn277dMzsmB1jOA2YQQqh5DAkAEtgaH0AM0edAB4yuEYAQUcjmIKzsMD2hRQbzgIXQLDwhAANL9-gAlADiwNI5DBcD8NQCZgAClYUhlsCoIF9YACgQA+ZFDPgASQAco5aP9WbAMCCcVRoMzqt1gNhaPtqbTHAzfkTUQB5IlY0EClAAIwAVtgNHTfvUEo8On9AY5EdAIDYVKLOnLFcCAGQOSKG8WFL62pXQR1sjlcnl03q-To0k1e6D1c7hppFVpGtGY-oAXjpgvZnO5vMdHsc-UG4jSGSyOXlmu1SCK70+2CJ1jGT2wEHqvz4BogWx2KAh2EZ+b4Fqyoq6MG6uR74n6kC6PWmQzm3DnPCEIgAYigUPC2EUNddbjxQJAYKv1zgEURsasYcg4Se2C4K9gPjp6nnxEu8WhT1EUHEtORkOVfkrHQKCFI8AEZOgJbtZkDIZ70fbBn0Ah8qxA-AjwAJk6AByNUrGwscZlgvh4KrJCmVeFDgNAtcAGZOlowjiOgUinxfPg30cDIYAwwwUGgbC3jXbCAIooSUHoDJIhQCgJgGZtDGdNsgL0GiUF4xMBPE7CZ3zOc+CIgZoAXbggA

💻 Code

class CustomElement extends HTMLElement {
  /** 
   * Useless constructor needed to workaround TS
   * `Base constructors must all have the same return type.`
   */
  constructor(...args: any) {
    super();
  }

  /* Extends Class.prototype and returns new Class */
  static define<
    CLASS extends typeof CustomElement,
    ARGS extends ConstructorParameters<CLASS>,
    INSTANCE extends InstanceType<CLASS>,
    PROPS extends object>
    (this: CLASS, source: PROPS & ThisType<PROPS & INSTANCE>)
    : CLASS & (new (...args: ARGS) => INSTANCE & PROPS) {
    return Object.defineProperties(
      this.prototype,
      source as any,
    ) as any;
  }
}

/** FooElement.js */

class FooElement extends CustomElement
  .define({
    /** Comment hover test */
    definedInFoo1: true,
  })
  .define({
    definedInFoo2: 'bar',
  })
  .define({
    definedInFoo3: 3,
  })
  .define({
    /** Sets 2 to 'foo' */
    fooMethod() {
      this.definedInFoo2 = 'foo';
    }
  }) { }

🙁 Actual behavior

Here's the outputed d.ts:

declare class CustomElement extends HTMLElement {
    /**
     * Useless constructor needed to workaround TS
     * `Base constructors must all have the same return type.`
     */
    constructor(...args: any);
    static define<CLASS extends typeof CustomElement, ARGS extends ConstructorParameters<CLASS>, INSTANCE extends InstanceType<CLASS>, PROPS extends object>(this: CLASS, source: PROPS & ThisType<PROPS & INSTANCE>): CLASS & (new (...args: ARGS) => INSTANCE & PROPS);
}
declare const FooElement_base: typeof CustomElement & (new (...args: any) => CustomElement & {
    /** Comment hover test */
    definedInFoo1: boolean;
}) & (new (...args: any) => CustomElement & {
    /** Comment hover test */
    definedInFoo1: boolean;
} & {
    definedInFoo2: string;
}) & (new (...args: any) => CustomElement & {
    /** Comment hover test */
    definedInFoo1: boolean;
} & {
    definedInFoo2: string;
} & {
    definedInFoo3: number;
}) & (new (...args: any) => CustomElement & {
    /** Comment hover test */
    definedInFoo1: boolean;
} & {
    definedInFoo2: string;
} & {
    definedInFoo3: number;
} & {
    /** Sets 2 to 'foo' */
    fooMethod(): void;
});
/** FooElement.js */
declare class FooElement extends FooElement_base {
}

🙂 Expected behavior

I expected FooElement_base to be shorter, smaller d.ts like this:

declare const FooElement_base: typeof CustomElement & (new (...args: any) => CustomElement & {
    /** Comment hover test */
    definedInFoo1: boolean;
    definedInFoo2: string;
    definedInFoo3: number;
    /** Sets 2 to 'foo' */
    fooMethod(): void;
});

It seems repeated structures aren't analyzed and flattened. I also don't see a way to flatten them myself. Changing CLASS to {[P in keyof CLASS]: CLASS[P]} will make each item reiterate every single property in HTMLElement in the d.ts.

This is a short reproducible, but actual code can get really heavy especially with mixins. I believe (but can't confirm), it causes the "Type instantiation is excessively deep and possibly infinite" error as well. There's probably a performance gain somewhere here as well. I know TS will flatten types, but not constructor return arguments. Maybe, simplified that's the core issue.

I have a second playground link that has a more verbose example including mixin patterns and extending classes extending classes:

https://www.typescriptlang.org/play?target=7&ts=5.2.0-dev.20230605#code/C4TwDgpgBAwgNgQwM5IPIDMA8AVKEAewEAdgCZJQDeUYATgPbCPgQBcUArsQNbH0DuxANxQAvgD4oAXijYhAWABQSgMaIUsDkiYBbAKJwIOksDyES5KAAlsAWQAyBoyapKoUAPQAqL1DfvfAFUkCEMNFXpibVoOFSZaKGIICFIUqCYofnpabgQGLlJZAGV-KF8AAwAhZGgIqOAYuOyKHS1TBDg4KAALBAA3aGBu6CQEYyhaCGAOWmJ0lgA6ctKvD3866Nj4gAoFvbyAcyR2BGIQAEpXRXd3JA5IWm3zhWuxJX9tBGAASxUzIjIOH+FgooEg9HQmm09H0hmMxGA4m2Q2+x1k51YuEopQmUxmczUyAoBABlhRFEoohe7lE71enx+fx033w32ImBxlQAgkU9MCyKCWBCobonPDgAAaHEAMQAcvzLLt9rQjiczgBtAC6lykklOIClr3cACU9NhAsb5SSQVBjXjZtgWJg5eJDTcoEVApUYPYeUUFRR4ESMJhTebLa6cVzjQBxf3WgWwSKbJq0AAKeTGUwgtCQmE93t9RSKkaNUG5vIA+gBJWVFbBc2UwPkJyzV+qnFQQR2QTAVvSl90Fn1+mt1htNlvmRPtz7ELs9iD5r0j4vicQ45HdVHsfsSqDM1nEdhyjEeldF-0AMnLPL0OJv2yS-CgSoWhzR0bjOskw8vY-rRtmygG9+wAidm0ubEy0maZZgPFk2S3VFnn8WllFebwoD0adLCDFAFjoRhmEgKBTkKWD8QoZ9YHUChVg+YAvl+KBUnQNklxxVd41wwVwUhGA2hhMUTDdG4vx40lA2TBotmyDNaCzIhc0wbjBxuWtAMnAMoFnJj527J01LE9w02NVA00km16AAIwAKwgOINzLZC0W4-ckHoGYu3YMyLOvWRtyQRdMD8yyQN08cgIHfd6DAH5kwAfl8vIfg6UKGAeUAABEICQFRaG+eLsnEc4cXYbiIqfCAXzfD92Akn9Iq04CbzCoooJxVB7Mc4AFjYji00ynMfjy7YcXcclCIYJgwQgEybm6hy4gWdAGH0BFCrGibFp6laTC2pBtk87yIHOBYdAQMBtm2dViCzfc+g6DgIG1aRJGg90vtxOC5nVHbvvu4wFu+9xPtB76SA4YxFJswx2CBiB1QABk1KAAEIpBkAByStsZBiGoDqdiDhmBA4bYdIYnmgHQaeuAXoJiH+EKpiKfYWSabLQmoD2BY4oSqIma+0RhfcTVqVB0RzgJmWcVQmD7TmckyIofVJfQ9ClG8XxpXoegRIRBY7PotYMMJDQ9YNuEXFbQMhNhZwEX8fqIHYpJtnB9wdaTHRxR6egBgSIhtDKM33QGpJSHbK2AEYOepsTpZdyOIE9nFU+j4grYAJnYbGbLyfG0LK15XfdtOvdYt2OKzq2AGZ2HrpPS-ccuOPTssfaKKYKBz9J6CgbH0H17Gw5xEf6FsKZunoUgniub6pszmP9f7nHJ+xyWaRLqg3gwn3qloQ2+pN8fzbo8s8hPnSrZP9uPfBlfiCP+PEg4TpDWlxeibooa4pGiAdsR8XjrBko0HYfNnxchVGiaIbIDhak6mWO4Dwnjb3SEFBYFskD-yykAl+eRpCJBqjAo4GslBa0UD7K2thELEGNqbJQ6AuBxG+JEKAtD6HbBUA7E+HMhQCT4TbBEyD3CUXgrw6EjtxQP0rhnGuUdV5T3oW-Gi2UvhpzlmWZ+XCjx5ygOgDoIQFq6P1nQo8jch7wOIAcYurxpYvCoYfBAAAve+Z8GIXyJFfdxIjTB2yvsffxF1uF6LZGIzwPg-BGiCCEMIFANiyVTCQlIaQMhZByHkLyZBigrCgFUGoRNwFyVzAeNoZFOg9H6IMYYUBRjjAkcrRYyxYnh2KfUCB2Q6qwNYPqJBP9bj3BzD0o4Csd7XGcdEo+Fi2SMPPiw+cgsgmzOIDw4RTtgACP4iKYS-jIlNKJhs2RqdO4R0USkYBeRVlv05qYi5WcZn0IMfXAArPciujzrn0KsdjGxdiW5OLpD7XKhJFI-AGB4ph1DonZXoHlYg2NTBJDSFoUIeVEklJSV41QMkoCgsQJMUgN8ZDVD8Zsl2rYngu0PEhJ5R5W681OVXZ+BK8gpBPm-AAHMjQFdIknVzBRy-xxD1GOUJcKzZ6ClCpCFcSkJZj6Cx2IZzF4sqJXys2XIuua9iEFyLi8DwHgoAAAFgBIAALQEEgHEK1tAGAJCQLPD+hQjHfDgDK8V7LNUnIeco+uxCjFwBCGqr1RL76T2nkMOe0rFDqu9ffZ+r9RUfzgKGuV98cF4MAVc2gCxDC2KGOmjVia-XZ3MaohYBwpgaKILG+N4aFVlvpWyHOCwmDShZCkbYOcFYNslb6z57Y2WNs2bHdt9BO34G7QrJQQA


Related:

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Jun 21, 2023
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 5.2.1 milestone Jun 21, 2023
@clshortfuse
Copy link
Author

I started committing types to github so I can see how they're extremely verbose.

Using 5.3.0-dev.20230823 doesn't seem to change much.

A good example is:

https://github.com/clshortfuse/materialdesignweb/blob/2c3ca4d6697d85824b4d2b6346667190df43175a/types/components/Divider.d.ts

declare const _default: typeof CustomElement & (new (...args: any[]) => CustomElement & {
    color: string;
    ink: string;
    typeStyle: string;
}) & (new (...args: any[]) => CustomElement & {
    color: string;
    ink: string;
    typeStyle: string;
}) & (new (...args: any[]) => CustomElement & {
    color: string;
    ink: string;
    typeStyle: string;
} & {
    vertical: boolean;
});
export default _default;
import CustomElement from '../core/CustomElement.js';
//# sourceMappingURL=Divider.d.ts.map

It should flatten to:

declare const _default: typeof CustomElement & (new (...args: any[]) => CustomElement & {
    color: string;
    ink: string;
    typeStyle: string;
    vertical: boolean;
});
export default _default;
import CustomElement from '../core/CustomElement.js';
//# sourceMappingURL=Divider.d.ts.map

There are also much, much larger files like:

https://github.com/clshortfuse/materialdesignweb/blob/2c3ca4d6697d85824b4d2b6346667190df43175a/types/components/Button.d.ts

That one comes out 594KiB. There's a lot of recursion and, though it works, will consume so much RAM it'll crash plugins (ESLint).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Investigation This issue needs a team member to investigate its status. Rescheduled This issue was previously scheduled to an earlier milestone
Projects
None yet
Development

No branches or pull requests

3 participants