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

Template Method Pattern does not work with @override #231

Open
mlhaufe opened this issue Feb 1, 2022 · 2 comments
Open

Template Method Pattern does not work with @override #231

mlhaufe opened this issue Feb 1, 2022 · 2 comments
Assignees
Labels
bug Something isn't working feature/override The override decorator
Milestone

Comments

@mlhaufe
Copy link
Contributor

mlhaufe commented Feb 1, 2022

import {Contracted, override} from "@final-hill/decorator-contracts"

@Contracted()
class A {
    constructor() {
        this.method()
    }

    method() {
        console.log('A.method()')
    }
}


class B extends A {
    @override
    method() {
        console.log('B.method()')
    }
}

new B() // No output
@mlhaufe mlhaufe added bug Something isn't working feature/override The override decorator labels Feb 1, 2022
@mlhaufe mlhaufe added this to the v0.23.1 milestone Feb 1, 2022
@mlhaufe mlhaufe self-assigned this Feb 1, 2022
@mlhaufe
Copy link
Contributor Author

mlhaufe commented Feb 1, 2022

Looks like the underlying issue is that super() is called (which then calls a method) before the feature can be restored to it's
implementation.

// Contracted.ts
abstract class InnerContracted extends Base {
    ...
    constructor(...args: any[]) {
        super(...args);
        ...
    }
    ...
}

TypeScript 4.6 is wanted here as it supports code before super() but this release is still in beta:

https://devblogs.microsoft.com/typescript/announcing-typescript-4-6-beta/#code-before-super

A near-term ugly alternative is to use an IIFE in the super call itself:

super(
    ...((...args) => { 
        ...
        return args
    })(...args)
)

In either case there is a challenge due to the this references in the current constructor body.

Is there an alternative approach through the @override stub?

//override.ts
...

// method decorators are evaluated before class decorators
return {
    enumerable: true,
    configurable: true,
    ...(!feature.isAccessor ? {writable: true} : {}),
    ...(feature.hasGetter ? {get(){ assert((this.constructor as any)[isContracted], MSG_NOT_CONTRACTED); } } : {}),
    ...(feature.hasSetter ? {set(_){ assert((this.constructor as any)[isContracted], MSG_NOT_CONTRACTED); } } : {}),
    ...(feature.isMethod ? {value() { assert((this.constructor as any)[isContracted], MSG_NOT_CONTRACTED); } } : {})
};

@mlhaufe
Copy link
Contributor Author

mlhaufe commented Oct 23, 2022

Looks like the semantics of decorators is changing soon (breaking change), so this fix will need to be delayed a bit:

microsoft/TypeScript#50820

@mlhaufe mlhaufe modified the milestones: v0.25.0, v0.26.0 Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature/override The override decorator
Projects
None yet
Development

No branches or pull requests

1 participant