cdklabs / cdk-monitoring-constructs

Easy-to-use CDK constructs for monitoring your AWS infrastructure
https://constructs.dev/packages/cdk-monitoring-constructs
Apache License 2.0
480 stars 70 forks source link

Extending MonitoringFacade breaks fluent interface #293

Closed naiello closed 1 year ago

naiello commented 1 year ago

Extending the MonitoringFacade class, then attempting to mix-and-match the built-in methods with the extended methods breaks the fluent interface due to type-check failures.

Setup:

export class MyMonitoringFacade extends MonitoringFacade {
  public monitorMyCustomThing(): MyMonitoringFacade {
    return this
  }
}

Repro:

const monitoring = new MyMonitoringFacade()
monitoring
  .addLargeHeader("Foo") // returns MonitoringFacade, NOT MyMonitoringFacade
  .monitorMyCustomThing() // error TS2339: Property 'monitorMyCustomThing' does not exist on type 'MonitoringFacade'.                                                                                                                                                                                                                                                 

This used to work prior to the fix for #284, since the TS compiler properly inferred the this return type.

Given that many non-TS JSII targets don't support this types, I'm not sure if this can be fixed while maintaining JSII support, but wanted to open the issue regardless to see if there's something I missed.

echeung-amzn commented 1 year ago

You're right, that was an oversight on my part. This actually does cause issues for us since extending the facade is a common pattern in another library we have.

I'll revert the change for now, but I'll reopen #284.

btd commented 11 months ago

In TS you can return this as result see playground

class BaseClass {
    public returnThis(): this {
        return this;
    }
}

class ExampleClass extends BaseClass { testThis() { console.log('ExampleClass') } }

let base = new BaseClass();
let base2 = base.returnThis(); 
console.log(base2 instanceof BaseClass)

let example = new ExampleClass();
let example2 = example.returnThis(); 
console.log(example2 instanceof ExampleClass)

example2.testThis()