Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eval causes problems in closure compiler #35

Open
wshager opened this issue Oct 15, 2013 · 8 comments
Open

Eval causes problems in closure compiler #35

wshager opened this issue Oct 15, 2013 · 8 comments

Comments

@wshager
Copy link
Contributor

wshager commented Oct 15, 2013

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
Copy link
Contributor Author

wshager commented Oct 21, 2013

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
Copy link
Member

we've all been told eval is evil... but i'm not sure we all agree that it's always evil 😄 (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
Copy link
Contributor Author

wshager commented Oct 21, 2013

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).

@wshager wshager closed this as completed Oct 21, 2013
@wshager wshager reopened this Oct 21, 2013
@adros
Copy link

adros commented Nov 12, 2013

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
Copy link
Member

@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
Copy link
Contributor Author

wshager commented Nov 12, 2013

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

@adros
Copy link

adros commented Nov 12, 2013

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

And yes, regexp fallback is need in IE.

@wshager
Copy link
Contributor Author

wshager commented Nov 12, 2013

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants