How to override the existing evaluators?

[Zvenigora]

Hi there,

Thank you for the nice code.

I wanted to override some existing evaluators via addEvaluator() function, but failed to. I tried both function and arrow function implementations. I also tried to inherit from class ExpressionEval but I do not know how properly handle static properties in the inherited class.

I do not know how to get access to context, isAsync or evalSyncAsync. Context and isAsync are marked as protected in ExpressionEval class and method evalSyncAsync is private. Moreover keyword this is undefined in the evaluator function in scrict mode. According to readme such the properties and methods should be visible in evaluator function in addEvaluator method.

Here is the sample code.

jseEval.addEvaluator('Identifier', (node: unknown) => { const self = this; return 100; });

Here are the variables in debugger.

Could you please provide a sample code for overriding the existing evaluators?

[6utt3rfly]

@Zvenigora Did you try something like this:

JseEval.addEvaluator('Identifier', function(node) {
 return node.test + this.context.string;
});

[Zvenigora]

@6utt3rfly I have a service which utilizes jsep_eval. It has tsconfig which defines strict mode.

The sample above does not pass the test.

It works though if strict mode is set to false. It seems JavaScript has no knowledge about private and protected modifiers in run-time. Typescript does not allow to use 'this.context' in strict mode and it ignores that when strict mode is false.

[6utt3rfly]

@Zvenigora - I can get further by adding this to the evaluatorCallback (i.e. directly editing the node_modules/jse-eval/dest/index.d.ts file), and making context public:

declare type evaluatorCallback = (this: ExpressionEval, node: AnyExpression, context: Context) => unknown;
context: Context;

However, trying to write:

JseEval.addEvaluator('Identifier', function(node: Identifier) {
  return this.context[node.name];
});

gives an error about name being in jsep.Identifier, but not in jsep.Expression. It then works if I change node type to any.

Jsep defines:

export type baseTypes = string | number | boolean | RegExp | null | undefined;
export interface Expression {
    type: string;
    [key: string]: baseTypes | Expression | Array<baseTypes | Expression>;
}

export interface Identifier extends Expression {
    type: 'Identifier';
    name: string;
}

Do you know a better way for JSEP (and Jse-Eval) to define these varying types?

[6utt3rfly]

@Zvenigora - I've also created PR #64 , but it might still require an update from my last comment?

[Zvenigora]

@6utt3rfly

Thank you for the updates.

From my understanding we need to pass reference to instance of ExpressionEval as parameter of evaluatorCallback. You mentioned that context was made public, so the last argument (context) is redundant.

Instead of declare type evaluatorCallback = (this: ExpressionEval, node: AnyExpression, context: Context) => unknown; It should be

declare type evaluatorCallback = (thisArg: ExpressionEval, node: AnyExpression) => unknown; The method addEvaluator might be written based on Discriminated unions. https://www.typescriptlang.org/docs/handbook/typescript-in-5-minutes-func.html#discriminated-unions

Instead of

JseEval.addEvaluator('Identifier', function(node: Identifier) { return this.context[node.name]; });

It should be something like

JseEval.addEvaluator('Identifier', function (thisArg: ExpressionEval, expr: AnyExpression) { if (node.type === 'Identifier') { const node = expr as Identifier; return thisArg.context[node.name]; } new Error(``Unknown node type ${node.type} was provided``); });

[6utt3rfly]

It seems that Typescript isn't smart enough to understand that the union is already discriminated at the callback ... unless using generics!

Now this actually works:

export declare type evaluatorCallback<T extends AnyExpression> = (this: ExpressionEval, node: T) => unknown;
static addEvaluator<T extends AnyExpression>(nodeType: string, evaluator: evaluatorCallback<T>): void;

JseEval.addEvaluator('Identifier', function(node: Identifier) {
  return this.context?.[node.name];
});

Hopefully that works for you @Zvenigora ? (see PR #64 update)

[Zvenigora]

@6utt3rfly ,

Oh, great! Yes, generics should work. It is really nice.

The only thing is missing. I am still not clear how the callback function gets the reference to value of this (instance of ExpressionEval). According to https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode#securing_javascript keyword this is restricted to undefined in strict mode. Does it get the reference to the instance of ExpressionEval implicitly based on the addEvaluator signature?

Thank you for the update.

[github-actions[bot]]

:tada: This issue has been resolved in version 1.5.1 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[6utt3rfly]

Thanks for reporting @Zvenigora and thank you for help with resolving it!

The call to the evaulator binds this to the class's instance (see L214: evaluator.bind(this)(node, this.context)). In strict mode, it was expecting this to be defined on the type, otherwise it didn't trust that it was set to be the class instance.

I also updated the code so it actually passes the context as the second argument, so that you could choose to use that over this if prefered (simplifies things so you can use arrow functions). Should be all included in 1.5.1, so please let me know if you still have troubles!