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

Local variables aren't working #1497

Closed
viatsyshyn opened this issue Apr 15, 2014 · 19 comments
Closed

Local variables aren't working #1497

viatsyshyn opened this issue Apr 15, 2014 · 19 comments

Comments

@viatsyshyn
Copy link
Contributor

Hi, there.

I have not updated to latest for a while. And after recent update I've discovered, that the following is not working any more

#some-id
  myLocalVariable = self.getSomeStuff()
  if myLocalVariable.isOk()
     .positive= myLocalVariable.getResult()
  else
     .negative= myLocalVariable.getError()

But the thing is, that if this syntax is forbidden. Why it does not trigger compile time error. Also it has really nice syntax highlight at online demo.

Here how it's compiled (online demo @ jade-lang.com)

function template(locals) {
    var buf = [];
    var jade_mixins = {};
    var locals_ = locals || {}, myLocalVariable = locals_.myLocalVariable;
    buf.push('<div id="some-id"><myLocalVariable>= self.getSomeStuff()</myLocalVariable>');
    if (myLocalVariable.isOk()) {
        buf.push('<div class="positive">' + jade.escape(null == (jade.interp = myLocalVariable.getResult()) ? "" : jade.interp) + "</div>");
    } else {
        buf.push('<div class="negative">' + jade.escape(null == (jade.interp = myLocalVariable.getError()) ? "" : jade.interp) + "</div>");
    }
    buf.push("</div>");
    return buf.join("");
}

Also it fails at runtime with following error

Jade:3
    1| #some-id
    2|   myLocalVariable = self.getSomeStuff()
  > 3|   if myLocalVariable.isOk()
    4|      .positive= myLocalVariable.getResult()
    5|   else
    6|      .negative= myLocalVariable.getError()

Cannot read property 'isOk' of undefined
@vendethiel
Copy link
Contributor

Are you compiling for client (this has changed) ? Also, be careful or = that must be unspaced (line 2)

@viatsyshyn
Copy link
Contributor Author

If I remove space. I still don't receive desired behavour

#some-id
  myLocalVariable= self.getSomeStuff()
  if myLocalVariable.isOk()
     .positive= myLocalVariable.getResult()
  else
     .negative= myLocalVariable.getError()

Compiles to

function template(locals) {
    var buf = [];
    var jade_mixins = {};
    var locals_ = locals || {}, self = locals_.self, myLocalVariable = locals_.myLocalVariable;
    buf.push('<div id="some-id"><myLocalVariable>' + jade.escape(null == (jade.interp = self.getSomeStuff()) ? "" : jade.interp) + "</myLocalVariable>");
    if (myLocalVariable.isOk()) {
        buf.push('<div class="positive">' + jade.escape(null == (jade.interp = myLocalVariable.getResult()) ? "" : jade.interp) + "</div>");
    } else {
        buf.push('<div class="negative">' + jade.escape(null == (jade.interp = myLocalVariable.getError()) ? "" : jade.interp) + "</div>");
    }
    buf.push("</div>");
    return buf.join("");
}

The thing is that with or without space, the result is not it was last year :)

Point is, if this was changed, why there is no error or warning at compile time?

@vendethiel
Copy link
Contributor

again -- I think the "undefined" error comes from the fact that you're not compiling it with client : http://jade-lang.com/command-line/

@viatsyshyn
Copy link
Contributor Author

http://jade-lang.com/demo/ - does this compile with --client ?

I also compile with jade.compile() API in web browser. I don't see any option equal to --client here http://jade-lang.com/api/ :(

@vendethiel
Copy link
Contributor

it's .compileClient

@viatsyshyn
Copy link
Contributor Author

Is following correct ?

jade.compile(content, {client: true, compileDebug: true, self: true})

Just to clear it. The error is there because myLocalVariable is not set with return of self.getSomeStuff(). But it was few version ago.

@vendethiel
Copy link
Contributor

No, you need to use the .compileClient function

@viatsyshyn
Copy link
Contributor Author

.compileClient returns string, .compile returns function

Nevertheles, both .compileClient and .compile should produce identical compiled code, or not?

myLocalVariable = self.getSomeStuff()

is compiled to

buf.push('<div id="some-id"><myLocalVariable>= self.getSomeStuff()</myLocalVariable>');

and gives no compilation error or warning. It compiled to something like

myLocalVariable = self.getSomeStuff()

few versions ago.

Also not a word about droping variable = value support in change logs

@vendethiel
Copy link
Contributor

Yeah, .compileClient returns a string - the function you can include in your .js
If that's not what you want (if you don't precompile your templates), then you're most probably not passing correctly the parameters.

EDIT : Just understood you expect it to be an assignment. use - var myLocal = self.foo() for that

@viatsyshyn
Copy link
Contributor Author

#some-id
  var myLocalVariable = self.getSomeStuff()

is compiled to

function template(locals) {
    var buf = [];
    var jade_mixins = {};
    buf.push('<div id="some-id"><var>myLocalVariable = self.getSomeStuff()</var></div>');
    return buf.join("");
}

Still not the result I want

@vendethiel
Copy link
Contributor

It's - var not var. This is just regular JavaScript.

@viatsyshyn
Copy link
Contributor Author

So the question is: Why few versions ago myVar = 5 worked and now it doesn't? Is this a bug or documentation issue? If it's a intended change, why this is not announced in change logs?

- var is an ugly syntax, and it's not highlighed properly. Still we have each and if statements, but could just write - for or - if instead.

@ForbesLindesay
Copy link
Member

Yes, sorry I didn't get to this sooner. The problem is simply that we removed support for first class variable assignment/declaration in v1.0.0 (see "Remove special assignment syntax" in the change log)

The fix is simply to use - var myVar = 5 anywhere you previously had myVar = 5.

The reasoning for the change was because the old syntax caused a great deal of confusion because people tended to assume it behaved as assignment when in fact it behaved as variable declaration. It was also tricky as it looked too much like <myVar>= 5</myVar> which is what it now compiles to.

@ForbesLindesay
Copy link
Member

In case it wasn't clear, any line that begins with - is treated as JavaScript.

@viatsyshyn
Copy link
Contributor Author

It will be a lot easier to get up-to-date if there were some migration warnings about syntax that were dropped or so. Or some checker tool that shows pit falls.

May be a good idea to add special var statement as an alias of - var, just to have more clean code

@ForbesLindesay
Copy link
Member

We have a change log, and we were pre v1.0.0. Now that we are post 1.0.0 we follow semver which means you will get a version that console.warns and then a major version bump (i.e. we move to at least v2.0.0) before the actual breaking change.

I'm not keen to add a var keyword. It only saves one character, so isn't a big benefit to cleanliness. Keeping things more explicit is very helpful with ensuring the language is easy to read. The fewer keywords you need to have learnt to read/write jade the better. Another thing to note is that I'm going to be very reluctant to add more keywords in general as they are all valid tag names in XML, which is a supported target for jade. On top of that, they could get added to the HTML spec, which would make it much harder to use jade to generate HTML.

@fritx
Copy link
Contributor

fritx commented Jul 9, 2014

Btw, why is function-typed options.locals(options.data) getting not working?
In grunt it worked, but now i use gulp version, it doesn't work.

@ForbesLindesay
Copy link
Member

@fritx options.data is a grunt specific feature. If you feel you need this functionality, perhaps try opening an issue on the jade gulp plugin.

@fritx
Copy link
Contributor

fritx commented Jul 10, 2014

Oic, thanks. But how about supporting it just in jade? 😃

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

4 participants