Skip to content

Add more strict types and better inference#11

Open
Kalin8900 wants to merge 1 commit into
engelmi:mainfrom
Kalin8900:strict-types
Open

Add more strict types and better inference#11
Kalin8900 wants to merge 1 commit into
engelmi:mainfrom
Kalin8900:strict-types

Conversation

@Kalin8900
Copy link
Copy Markdown

@Kalin8900 Kalin8900 commented May 18, 2023

Hello everyone.

I would like to propose an extension to the current solution to introduce better type inference and type safety.

Please let me know what you think. If you find it useful, I will update the documentation.

@Kalin8900 Kalin8900 requested a review from engelmi as a code owner May 18, 2023 13:13
@Kalin8900 Kalin8900 changed the title feat: add more strict types and better inference Add more strict types and better inference May 18, 2023
@engelmi
Copy link
Copy Markdown
Owner

engelmi commented May 22, 2023

Hi @Kalin8900, thank you for your contribution! I am currently a bit swamped with work, but I'll definitely have look at it the next days.

@engelmi
Copy link
Copy Markdown
Owner

engelmi commented Jun 19, 2023

Hi @Kalin8900,
what do you want to achieve with the type-safety and where do you want to have it?
In general, I like the idea of using generics to pin down the types. But at the same time the API gets quite a bit more complex, I think. And, although this seems feasible by using a decorator, when using a more general Pointcut, e.g. .*, it is not applicable.

So I am wondering if it is sufficient for your use case to have the "type-safety" on the AspectContext and not type-check if the method the aspect is applied on has the required parameter/return/error types.
So what we could do is sth like this:

// make the Aspects context generic with the default AspectContext 
export interface Aspect<T extends AspectContext = AspectContext> {
    execute(ctx: T): any;
}

export interface AspectContext {
    target: any;
    methodName: string;
    functionParams: any[];
    returnValue: any;
    error: any;
};

// ...

// specify a custom context with a number as return type
interface SpecificAspectContext extends AspectContext {
    target: any;
    methodName: string;
    functionParams: any[];
    returnValue: number;
    error: any;
}
class SpecificAspect implements Aspect {
    execute(ctx: SpecificAspectContext) {
        executeHasBeenCalled = true;
        ctx.returnValue = 2;  // works
        ctx.returnValue = "2"; // error
    }
}
class YetAnotherSampleClass {
    @UseAspect(Advice.After, SpecificAspect)
    public doIt(): number {
        return 1;
    }
}

This way you can pretend to work with specific types in the Aspects execute, but since we don't evaluate it on the decorator, it might be off and the user/developer needs to be careful still. What do you think?

@Kalin8900
Copy link
Copy Markdown
Author

Hello @engelmi!

The main thing I would like to achieve is having the type check on the decorator level. I don't see a point of adding it only on the Aspect level, because you can have unexpected behaviors then. Maybe there is a way to combine the decorator level check if specifying custom context?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants