raycmorgan/Mu

eval(code) in compiler fails

djui opened this issue · 9 comments

djui commented

I tried to run the following code:

mu.render("index.html", {host: IOHOST}, {}, function(err, buffer) {httphelper.sendHTML(res, 200, buffer)})

Then "err" has the following error: "SyntaxError: Unexpected identifier"

It fails in compiler.js at "var func = eval(code);" with code:

(function COMPILED(context, options) {
options = options || {};
var Mu = M;
var REE = new RenderEventEmitter(options);
process.nextTick(function () {
REE.write(Mu.escape(Mu.normalize(context, "host")));
REE.close();
});
return REE;
})

I checked that the template file is read correctly, I tried the example, also fails.
I use the latest NodeJS (1.92) and Mu.

Thanks, I will look into this and fix it ASAP. As a side note I am currently rewriting some of the internals of Mu to be more awesome which removes the use of eval.

Thanks!

Are you successfully able to run the test.js?

I see intermittent errors with "RenderEventEmitter" undefined when compiling templates - not sure if this is related (but would be fixed w/out eval). This happens sporadically when caching is disabled. I think I must be doing something wrong. I can send any debugging info you need. Thanks!

pibi commented

@ithayer
I've solved this by adding a reference to RenderEventEmitter in compile function in compiler.js. You can find my modified version here

Thanks! I changed my version to use Script.runInNewContext instead of eval, which might be better depending on how one feels about eval.

http://gist.github.com/459172

edbo commented

@raycmorgan Can I ask what was the original reason for running it using an eval? Was there a specific problem you were encountering that required the need for this block? THe reason I ask is that I might have a go at re-writing that myself (given enough time).

If you're interested, you can see my fix here (just a few lines):
http://github.com/limbolabs/Mu/commit/168e5e4d8c8af2544e7b3637e289fc62e633e5a8

edbo commented

@ithayer Yeah I have seen your solution which is definitely an improvement, but I was hoping that there would be a way to do this without any javascript code written within a string.

This should be resolved in the new version of Mu, just now getting around to closing the issue.