TomFrost / Jexl

Javascript Expression Language: Powerful context-based expression parser and evaluator
MIT License
561 stars 92 forks source link

Does not handle evaluating when data properties actually include dots #70

Closed austinkelleher closed 4 years ago

austinkelleher commented 4 years ago

First, I'd like to say how much I appreciate all of the hard work on this module. It's so awesome!

I ran into a case where the data I am passing to be evaluated actually includes dots in the properties.

Example:

const jexl = require('jexl');

const evaluated = jexl.evalSync('my.name', {
  'my.name': 'Austin'
});

console.log(evaluated); // undefined

Is this something we should try to support? I haven't dug into the code much yet, but I could help work on making this change if it is something that we think is possible without too much effort.

TomFrost commented 4 years ago

Hi Austin, thanks for your kind words!

My approach to contexts in Jexl was to keep parity with Javascript -- both for intuitiveness, and because if you dive into a parsing mechanics, the restrictions around variable naming in javascript are there for super good reasons.

So the way to support your use case -- whether it be in Jexl or in raw JS -- would be to make your dotted-name variable an object property. That would look like:

const jexl = require('jexl');

const evaluated = jexl.evalSync('vars["my.name"]', {
  vars: { 'my.name': 'Austin' }
});

console.log(evaluated); // Austin

Since you can't have my.name as a top-level variable in JS either, I think that's sane. Since reading your post I've been considering what it might look like to support this in Jexl, but consider a context like the below:

{
  "my.name": "austin",
  my: {
    name: "tom"
  }
}

Specifying which of those you want would require injecting additional syntax characters to Jexl, or prefix the dot with an escape character of some sort... and at that point, I think having free-form property names nested in a container object is probably the cleaner and more obvious solution anyway.

If you had any other implementation thoughts, I'm all ears -- but I think it would have to offer wide-reaching value to warrant breaking JS parity.

austinkelleher commented 4 years ago

This makes complete sense, and I appreciate your response. I have a few other proposals that I will open up in separate issues. Thanks again!