persvr / rql

Resource Query Language
http://www.persvr.org/
268 stars 49 forks source link

Eval causes problems in closure compiler #35

Open wshager opened 10 years ago

wshager commented 10 years ago

The problem is in js-array.js line 401: op() is no longer referenced correctly after the function name is shortened.

It would be really useful to rewrite rql/js-array with a whole lot less eval where possible...

wshager commented 10 years ago

Ok so I have a fix, but I'm positive that the same functionality can be achieved without using eval, which as we all know is evil...

neonstalwart commented 10 years ago

we've all been told eval is evil... but i'm not sure we all agree that it's always evil :smile: (take a look at https://github.com/felixge/faster-than-c#examples for example)

i'm curious how you think it could be done without eval - not that i don't think it could, but wondering how you would approach it.

wshager commented 10 years ago

Lately I've been working on a concatenative dsl based on dojo. Once the file is parsed into an object I never have to use eval, as everything is either a single function, or a for-loop of function calls.

Since the object here is to chain functions, I think a similar approach can be used. The array could just be threaded along. Do you think this would impact performance much?

Perhaps my library would benefit from eval. Imo Felix merely points out that eval is great for slapping together huge quantities of data (wrapping it with new Function).

adros commented 10 years ago

I have created a (temporary) fix, where name of the function "op" is not hardcoded in string, but dynamically computed:

return "(function(){return " + fnName(op) + "('" + value.name + "').call(this" +

where function fnName is defined as:

function fnName(fn) {
    return fn && (fn.name || fn.toString().match(/^function\s*([^\s(]+)/)[1]);
}
neonstalwart commented 10 years ago

@adros have you seen #37? also, is there a case where fn.name doesn't work? i don't know of a need to use the regular expression in this case.

wshager commented 10 years ago

This is correct, IE is oblivious to function.name.

adros commented 10 years ago

Sorry #37 was not mention here, so I have missed it.

And yes, regexp fallback is need in IE.

wshager commented 10 years ago

I would opt for a function.name shim, however.