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

Unexpected behavior when trying to initialize local variables #92

Open
ChrisMuc opened this issue Dec 17, 2012 · 8 comments
Open

Unexpected behavior when trying to initialize local variables #92

ChrisMuc opened this issue Dec 17, 2012 · 8 comments

Comments

@ChrisMuc
Copy link

I'm using multiple ejs templates which all include a common header tamplate, whereas the header template should be customizeable by the parent templates. Therefore, I'm using the following (simplified) structure:

index.ejs:

<% var showMenu = true; %>
<% include header%>
CONTENT

header.ejs:

<% if (typeof(showMenu) === 'undefined') { var showMenu = false; } %>
HEADER
showMenu:<%= showMenu %>

Unfortunately, the first line in header.ejs ends up in an unexpected behavior / bug!
The idea behind this line is to check whether the parent template uses customization, and if not, initialize customization-variables with defaults. But in this small example showMenu is always set to false! Just like showMenu has never been declared in the parent template!

More interestingly:
When you swap the first line in header.ejs to:

  typeof(showMenu):<%= typeof(showMenu) %>

to see if typeof() really returns 'undefined', you wil get surprised again. Instead of 'undefined' it returns the previously expected value ('boolean'):

  typeof(showMenu):boolean
  showMenu:true

So technically in our first example, showMenu should not be set to false.

If you remove the first line in index.ejs, you will also see an 'undefined'. (To actually see this value, you also have to remove the last line in header.ejs, as otherwise you will get an undefined exception)

If you are using a header.ejs like this:

typeof(showMenu):<%= typeof(showMenu) %>
HEADER
<% if (typeof(showMenu) === 'undefined') { %>
showMenu:not-set-by-parent
<% } else { %>
showMenu:<%= showMenu %>
<% }%>

everything works fine. Anyway, I would prefer the first version to work for mainly three reasons:

  • It allows me to manage defaults in on place (and not scattered across the whole template)
  • I'm also using it for other variabels like "activeMenuItem" where not using the posibility to set a variable to an default value, may cause redundant lines.
  • and most important: "if (typeof(showMenu) === 'undefined') { var showMenu = false; }" simply does not behave as expected; nor does it throw an exception to inform the user about doing something wrong!

And just in case: I don't wanna start a discussion about MVC principles. I know that logic belongs to the controller and not to templates! But in my opinion this defnitely is a bug; in particular as the codes does not behave as expected.

@ChrisMuc
Copy link
Author

As a quick fix, users can prevent the issue by using a local object:

header.ejs:

typeof(showMenu):<%= typeof(showMenu) %>
<%
    var local = {};
    if (typeof(showMenu) === 'undefined') {
        local.showMenu = false;
    } else {
        local.showMenu = showMenu;
    }
%>
HEADER
showMenu:<%= local.showMenu %>

Anyway, the bug should be fixed!

Cheers

@ForbesLindesay
Copy link
Contributor

Could you put your code in code blocks to make it easier to read in GitHub issues. Just put 'html' or 'javascript' on its own line before your code and '```' on its own line after your code.

@robdodson
Copy link

+1 for this. it's totally weird

@netpoetica
Copy link

Possibly, this answer may be too simple, or I may not see the problem. But if you have index.ejs and you declare a variable and set it to true, it's type is now "boolean". For example, in JS console type

typeof(true)
> "boolean"

So if you call header afterwards, your variable showMenu is not "undefined", it's "true". This is why your code

 typeof(showMenu):<%= typeof(showMenu) %>

has a typeof "boolean".

If you want to manually override the showMenu variable, you will have to test if its value is true instead of typeof undefined - because it is defined.

<% if (showMenu) { var showMenu = false; } %>
HEADER
showMenu:<%= showMenu %>

Again, it's possible I am misunderstanding the issue here.

@robdodson
Copy link

No that's not the issue. The problem is that a variable declared in a parent template is available in a child template only in a really bizarre fashion.

For instance, if you do this:

foo.ejs

<% var hello = 'world' %>
<% include partials/bar.ejs %> 

bar.ejs

<%= hello %>

Then 'world' will show up in your bar.html file when it's rendered. So far so good. But if you do this:

foo.ejs

<% var hello = 'world' %>
<% include partials/bar.ejs %> 

bar.ejs

<% var hello = hello || 'busted' %>
<%= hello %>

Then the value of hello will be 'busted'. Which doesn't seem right. If you do this in the same template:

<%
var hello = 'world';
hello = hello || 'world';
%>

then the value of hello will still be 'world'. It's just that whenever you do a partial this seems to get screwed up. As a result it's really hard to provide a safe default value for a variable.

@robdodson
Copy link

Also it seems like the only way to get around it is to use typeof.

Neither of these work:

var stylesheets = stylesheets || [];
var stylesheets = stylesheets ? stylesheets : [];

This is the only solution that works for me:

var local = {};
if (typeof(stylesheets) === 'undefined') {
  local.stylesheets = [];
} else {
  local.stylesheets = stylesheets;
}

@netpoetica
Copy link

@robdodson - for the typeof thing, see #90

For why all this strangeness is happening, I look forward to someone way smarter than me telling us why :) my speculation is that it has something to do with the fact that you're trying to declare a local variable with the same name as a global variable, and somehow the EJS engine managing var declarations in a way that results in var hello = hello || 'world', hello being world.

There is this super-genius, magical parse function https://github.com/visionmedia/ejs/blob/master/lib/ejs.js#L116 which uses an array of strings to turn your template into a JS function (I think). I wonder if somehow scope gets lost in there

@Mart-Bogdan
Copy link

I don't think you should use sutch code.
All point of EJS is to separate logic from View, you should move that variable to model and pass it as parameter when you are applying ViewEngine.

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

5 participants