Closed Zvenigora closed 10 months ago
Hi @Zvenigora ! Thank you for your PR! I'm truly sorry for taking so long to respond!
For case-sensitivity, I'm wondering if it might be better to simply make a plugin that overrides the Identifier
handler (and maybe other code should then call that handler instead of directly looking at the node.name
so it can work in all places)?
For function binding, what kind of scope are you currently passing into the evaluator? Couldn't the binding be done by the caller like this:
eval('foo(arg)', { foo: a => this.foo(a) });
// or:
eval('foo(arg)', { foo: this.foo.bind(this) });
I'm just trying to flush out the rationale/use-case before merging anything, if you can help 🙂
Hi @6utt3rfly, Thank you for the reply!
About case-sensitivity. Yes, the feature may be implemented by duplication and overriding of plugins. It has both pros and cons. The code would be cleaner, but it needs to override not one but three plugins (Identifier, Literal and Member) and it breaks DRY principle.
Primary purpose of Options is not limited to case-sensitivity. Options are designed to change the Evaluator run-time behaiviour.
About the function-binding. I like your brilliant idea to use the JavaScript arrow functions for that. Thank you for sharing this. However as it turns out I am not clear how to make it works. I made a simple test without extra arguments (arrow-function-bindings.test.js) and it fails. Might be I broke something in the Evaluator code or did not call the bind function.
I am not sure how the PR system works. Since this PR I made several additons. Please take a look to the test in my forked repository for the scoped bind functions (scope-function-bindings.test.js). It works. The test immitates a few external services (cat, dog and counter) and they are controlled through Evaluator expressions. Each service has own this
reference and may accept the extra arguments.
Happy Thanksgiving!
@Zvenigora - Thank you for those examples! I think there may be some other ways to bind your expected this
without changing the jse-eval source code... but I believe you're right that it's not setting this
correctly in some cases.
If I change this line to:
.apply(caller === node.callee ? this.context : caller, args))
then I believe that will work correctly for member expressions like "string".slice()
(where this
has to be set to the caller string), and also for your example function() { this.xxx}
. Any thoughts?
Hi @6utt3rfly, Thank you for the reply!
I am sorry to delay the answer. It seems that you have already made and committed the change for reference of this
. Frankly speaking I do not have much to add. You know your code much better than me.
As for me, I spent some time incorporating the evaluator code into my POC and stuck for a while in the attempts to figure out how to pass extra parameters into the functions defined in the context. I found that in addition to function binding JavaScript supports many other ways to do the trick including but not limited to a factory function with closure and currying.
I still hope you could find the code of this PR useful.
This PR is closed.
Hi @6utt3rfly,
This PR adds
options
to Evaluator. Options is a configuration object which allows to update run-time behaviour of the Evaluator without the code changes. Such the configuration may allow or block some features, or completelly change them. For example, it can prevent call to eval function, add scoped contexts, add function aliases, permit side-effect (non-pure) functions or disable some of internal evaluators.This PR implements options for a case-insensitive evaluation. While JavaScript is the case-sensitive language some target audience finds it hard to use. To provide the case-insensitive evaluation, set caseSensitive to false (default is true).
If you find the PR is useful, please ignore package.json and changelog.md during the merging. I had some troubles with release version support, because it autoincrements via workflows.