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

test/ejs.js fails with ejs.0.8.4 #90

Closed
hpaulj opened this issue Jun 15, 2013 · 9 comments
Closed

test/ejs.js fails with ejs.0.8.4 #90

hpaulj opened this issue Jun 15, 2013 · 9 comments
Labels

Comments

@hpaulj
Copy link

hpaulj commented Jun 15, 2013

test/ejs.js gives an error

1) class EjsEngine < Template Asset [.jst.ejs] #compile() "before all" hook:
 Uncaught TypeError: Object #<Object> has no method 'asset_data_uri'

This is with a newly installed mincer. It runs ok if I install the previous version of ejs.

npm install ejs@0.8.3

So evidently there is some incompatibility with the latest ejs (0.8.4).

@hpaulj
Copy link
Author

hpaulj commented Jun 16, 2013

As best I can tell this error is because the new ejs wraps the template buf in a function evaluation

var buf = [];
with (locals || {}) {
 buf.push('');__stack.lineno=1; this.id = 1 ; buf.push('\n<header id="', escape((__stack.lineno=2,  "header" +     this.id )), '">\n    <p>', escape((__stack.lineno=3,  this.text )), '\n  <img src="', escape((__stack.lineno=4,   this.asset_data_uri("header.jpg") )), '" title="beautiful header" />\n</header>');
}

is now

var buf = [];
with (locals || {}) { function(){
 buf.push('');__stack.lineno=1; this.id = 1 ; buf.push('\n<header id="', escape((__stack.lineno=2,  "header" + this.id )), '">\n    <p>', escape((__stack.lineno=3,  this.text )), '\n  <img src="', escape((__stack.lineno=4,  this.asset_data_uri("header.jpg") )), '" title="beautiful header" />\n</header>');
})();
}

It's the result of tj/ejs#79 which is supposed to "Allow named function in views"

@ixti
Copy link
Collaborator

ixti commented Jun 16, 2013

Yeah, the problem came out of this change. But I have a strong believe that it is caused by lazySource not being playing nice with this change. So I'm gonna drop lazySource to avoid this. lazySource were introduced to allow engines generate different things (JST vs really pre-processing) but I have a feeling it can be resolved much easier.

@puzrin
Copy link
Contributor

puzrin commented Jun 18, 2013

We decided, that lazy-loading feature implementation is not good, and makes too many problems with archtecture. So, it will be removed. I've disabled appropriate tests.

PS. First pass still failing, second is ok. To reproduce - remove ./test/assets folder before runing tests.

@hpaulj
Copy link
Author

hpaulj commented Jun 21, 2013

The problem isn't just the lazy-loading. A simple EJS.render(source, options) fails if the template is using this.
I can get this test to work by changing ejs/lib/ejs.js to produce

var buf = [];
with (locals || {}) {(function(){
 buf.push('');__stack.lineno=1; this.id = 1 ; buf.push('\n<header id="', escape((__stack.lineno=2,  "header" + this.id )), '">\n      <p>', escape((__stack.lineno=3,  this.text )), '\n  <img src="', escape((__stack.lineno=4,  this.asset_data_uri("header.jpg") )), '" title="beautiful header" />\n</header>');
}).call(this);
}

In other words, pass the scope through this new function layer.

I submitted tj/ejs#126 for this issue (6/24)

@hpaulj
Copy link
Author

hpaulj commented Jun 21, 2013

With this async compile, how are (or were) local variables supposed to be passed to the template? For example, if in this test, this.text was replaced with fig.text, how would fig get passed through? Or is this a moot issue with the demise of lazy-loading?

So far the best I can do is use a helper function.

@ixti
Copy link
Collaborator

ixti commented Jun 21, 2013

Thanks in advance. Although lazySources are adding it's own problems, so I was able to remove some problems with cleaning them out - I have a better plan how to re-introduce ability to use EJS for both server-side preprocessing and for JST.

@ixti
Copy link
Collaborator

ixti commented Jun 21, 2013

Hmm. I tend to think you're right. We pass something like this to EJS: { scope: context, locals: locals }. Locals are holding only helper functions, so if you'll add a helper named foo you'll be able to use it as foo() from the templates. Variables can be assigned to context: env.ContextClass.prototype.foobar = 123, but as far as I can see EJS use scope as this of rendered function...

@ixti
Copy link
Collaborator

ixti commented Jun 21, 2013

Probably it worth to rework the way we prepare locals to respect properties, not only helper functions...

@puzrin
Copy link
Contributor

puzrin commented Jun 27, 2013

Solved in master, because lazy-stuff removed

@puzrin puzrin closed this as completed Jun 27, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants