TomFrost / Jexl

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

Strange Behavior with number as string as the needle #16

Closed dannyrscott closed 8 years ago

dannyrscott commented 8 years ago

Terrible title aside, here's some weirdness. Is it a bug? Hard to say, but it is odd.

'use strict';
var jexl = require('jexl'),
    Bluebird = require('bluebird'),
    haystack = {
        foo: 'bar',
        something: 'else'
    };

function jexEval(needle) {
    return jexl.eval(needle, haystack)
        .catch((err) => {
            return `Error: ${err.message}`;
        });
}

Bluebird.props({
    '51.5': jexEval('51.5'),
    51: jexEval(51),
    'foo': jexEval('foo'),
    'hello': jexEval('hello')
})
.then((res) => {
    console.log(res);
//{ '51': 'Error: str.split is not a function',
//  '51.5': 51.5,
//  foo: 'bar',
//  hello: undefined }
});

Tested with jexl 1.1.2/3 on node 4.4.3

TomFrost commented 8 years ago

Very interesting! Looking into this.

TomFrost commented 8 years ago

Oh, apologies -- I thought it was choking on the 51.5 string and I just re-read. jexl.eval's first argument must be a string, so I'm not heartbroken that it doesn't work, however that error message is extremely cryptic. I'm working on a 2.0 release now and will consider either casting all input to a string, or delivering a better error message for that issue. Keeping this open in the meantime.

dannyrscott commented 8 years ago

This is actually not about the error, but the fact that '51.5' evaluates to '51.5' instead of indefined

TomFrost commented 8 years ago

Why would it be undefined? 51.5 is a number.

dannyrscott commented 8 years ago

Is that the expected behavior? Resolve to the number if the input string is a number? This seems odd with how hello evaluates to undefined (since their is no 'hello' key).

dannyrscott commented 8 years ago
var haystack = {
    '0': 'Something',
    '2': 'Something else'
};

jexl.eval('0', haystack)
    .then((res) => {
        console.log(res); //'0'
    });
TomFrost commented 8 years ago

hello is an identifier, so it tries pulling it from the context object. When you call jexl.eval('2+2'), Jexl isn't first looking for the string 2 as a key in the context object and then using the number 2 only if it's not found -- it recognizes that 2 is a number immediately, and passes it to the binary operator +.

So by extension, jexl.eval('2') would of course just resolve with the number 2 regardless of the context, exactly how jexl.eval('-2.5') results in -2.5, jexl.eval("foo") results in "foo", and jexl.eval("51.5") results in 51.5.

That does cause an issue with accessing numeric keys in the top-level of the context, but consider the following javascript file:

var 0 = 'Something';
console.log(0);

That's essentially what you're trying to replicate by accessing a variable with a numeric name at the top level. This can only be done if the numerically-named variable is a child of some other property, in which case you can access it as a quoted string.

TomFrost commented 8 years ago

Illustrating the above -- if you have an object with numeric keys, this is a great solution:

var context = {
  numericKeys: {
    '0': 'foo',
    '1': 'bar'
  }
}

jexl.eval('numericKeys["0"]', context).then(console.log) // "foo"
dannyrscott commented 8 years ago

Makes sense and works for me. I've never run into a problem with it directly but it might be worth pointing out as a potential "gotcha".

jexl.eval('foo') results in undefined though

TomFrost commented 8 years ago

I'm not sure I'd really consider it a gotcha, as it acts exactly like JavaScript itself. Drop into the Node CLI and foo gives you undefined, a number will give you the same number. Try to define a variable that starts with a number and you simply can't.

The execution context you provide to Jexl is exactly that -- the environment in which your code will execute in. So top-level numerics are just as inaccessible there as they would be in JS itself. A good proof of concept here is Node's VM object; set a numeric key at the top level of a sandbox object and you'll experience the exact same thing: https://nodejs.org/dist/latest-v4.x/docs/api/vm.html#vm_vm_createcontext_sandbox