Jinja-style tests

[noahlange]

Summary

I've got most of the docs changes ready to go, but I figured this would be something worth discussing before I spend time polishing them.

• adds ~20 Jinja2-style boolean tests + aliases
• updates node list, parser and compiler to interpret and properly compile Is nodes
• adds tests for tests, parser and compiler changes/additions (no changes to existing tests)
• adds addTest/getTest methods to the Environment
• CAVEAT: changes lexer to interpret "null" as the primitive null instead of a lookup to "null" in the context. this was necessary to make the test null is null pass — this may be a compatibility issue
• CAVEAT: adds soft dependency on ES6 Symbols, Maps and Sets for full functionality of iterable and mapping tests. The whole thing shouldn't explode / degrades gracefully if they're not available, but I don't currently have the ability to test that in an non-evergreen environment, so I can't say if we'd still have IE8 compat.

Addresses #532, #879 and #1015 .

Checklist

I've completed the checklist below to ensure I didn't forget anything. This makes reviewing this PR as easy as possible for the maintainers. And it gets this change released as soon as possible.

• Proposed change helps towards purpose of this project.
• Documentation is added / updated to describe proposed change.
• Tests are added / updated to cover proposed change.
• Changelog has an entry for proposed change (if user-facing fix or feature).

[fdintino]

This is fantastic, thank you! I have no feedback because everything here looks perfect, down to following the inconsistent indentation level between different files (As an aside, I'd love to fix that at some point, probably settling on 2 spaces since that seems to have become a de facto standard in node projects; the only reason I've held off is concern over merge conflicts with PRs).

[noahlange]

Happy to help!

Thoughts on the caveats? - I'm mostly concerned about the compatibility ramifications of a new null primitive. If someone had decided to use null as a variable in their templates I imagine they could expect some breakage.

Also, one last thing I forgot to mention - I don't know Jinja's operator precedence; I stuck it between in and the comparison operators. If that's the wrong place, let's fix it.

[ArmorDarks]

changes lexer to interpret "null" as the primitive null instead of a lookup to "null" in the context. this was necessary to make the test null is null pass — this may be a compatibility issue

So, this means that something like this is no longer legit?

{% set null = 'Hey' %}

adds soft dependency on ES6 Symbols, Maps and Sets for full functionality of iterable and mapping tests. The whole thing shouldn't explode / degrades gracefully if they're not available, but I don't currently have the ability to test that in an non-evergreen environment, so I can't say if we'd still have IE8 compat.

Well, this sounds a bit bad.

From one point, it is fixed by babel. It is used almost everywhere anyway.

To make it even less important, for frontend React or Vue definitely a better candidate. We tried to use Nunjucks in past, but it seems to be messy and unreasonable.

However, from another point, if we want to make it "right", it should be a optional thing. Want this functionality and can sacrifice IE support or use babel? Enable it. Otherwise leave it off. Or, in this case, I'd say that it should on by default, but have an option to disable for older browsers.

As an aside, I'd love to fix that at some point, probably settling on 2 spaces since that seems to have become a de facto standard in node projects; the only reason I've held off is concern over merge conflicts with PRs

standard with standard --fix flag or Prettier are definitely candidates for easy-fixing this. But yeap, it will screw completely PRs.

[noahlange]

@ArmorDarks -

Re: null - that is correct. The alternative is having the following evaluate to true:

{{ null is not null }}

...which I'd imagine to be fairly confusing to most users who don't have a pretty good understanding of Jinja + Nunjucks semantics. Pick your poison.

Naturally, I'd be happy to postpone that semantic change for a major version bump.

Re: IE8 support, I'm probably being overly cautious about potential compatibility issues - the "problems" are with two runtime test calls, to iterable and mapping. Both should degrade gracefully without any changes in a non-ES6 environment, and I've tried my best to include some "best guess" alternatives for those. I just don't have the ability to test it because I have no clue how to get ahold of an IE8 VM.

Long-term, I think Babel + ESLint + AirBNB + Prettier are a pretty solid bet. But that's probably best left to another issue.

[ArmorDarks]

Re: null - that is correct. The alternative is having the following evaluate to true:
..which I'd imagine to be fairly confusing to most users who don't have a pretty good understanding of Jinja + Nunjucks semantics. Pick your poison.

I think the answer to this thing is quite easy — how does it behave in Jinja2? And yeap, since it is potentially breaking change, a major bump needed.

Re: IE8 support, I'm probably being overly cautious about potential compatibility issues - the "problems" are with two runtime test calls, to iterable and mapping. Both should degrade gracefully without any changes in a non-ES6 environment, and I've tried my best to include some "best guess" alternatives for those. I just don't have the ability to test it because I have no clue how to get ahold of an IE8 VM.

Okey, looked into the code. Fallbacks seems to be reasonable. I only hope that we didn't miss any edge cases.

I'd say that normally if you try to use iterable or mapping when Symbol or Set is unavailable, Nunjucks should simply throw a error with explanation why it does not work (built-in error "as it is" will be misleading), otherwise it is very unsafe. But unless iterable or mapping are unused in templates, you should be ok to go within IE8.

However, in those cases without Symbol indeed only String and Array should be iterable, and when there is no Set there is no point in ensuring that it isn't Set in case of mapping. So, seems to be ok as it is.

[ArmorDarks]

Mm, regarding iterable. I think there is an error there. Object (dict) will be treated as non-iterable. However, for Nunjucks it should be an iterable, since you can iterate upon it in Nunjucks. This behavior should be same as in Jinja2, where dict will be iterable true too, if I'm not mistaken.

[ArmorDarks]

Also, will callable work for macros? Worth adding a test.

[noahlange]

I think the answer to this thing is quite easy — how does it behave in Jinja2? And yeap, since it is potentially breaking change, a major bump needed.

My understanding is that it treats null as a variable, because the null in JS is none in Python. I don't know if we want to strictly maintain that behavior or defer to JS semantics. Will leave that one up to @fdintino.

Mm, regarding iterable. I think there is an error there. Object (dict) will be treated as non-iterable. However, for Nunjucks it should be an iterable, since you can iterate upon it in Nunjucks. This behavior should be same as in Jinja2, where dict will be iterable true too, if I'm not mistaken.

Great. I can add that.

Also, will callable work for macros? Worth adding a test.

It does. We can add a test.

[ArmorDarks]

Great. I can add that.

Hey, don't take my words for granted! :) I'm not that experienced with compilers, and didn't work with Jinja2. It is only my assumption that in Jinja and Nunjucks any dict should be treated as iterable...

[fdintino]

I would imagine that anyone still supporting IE 8, but using open-source javascript libraries, has polyfills in place for a lot of newer browser features. And they're also probably fairly conservative about upgrading libraries. My inclination is to not worry too much about it, and address it if we inadvertently break support and someone opens a ticket. It might very well be the case that nobody would actually be affected, since barely anything else works in IE 8 out-of-the-box anymore.

With regard to null: this too seems like something that would be easily fixed on the user's end, and the clarity of their code would surely benefit as a result. That said, your contributions @noahlange might inspire me to spend a bit more time on this library, in which case we could roll these features and others into a major version bump.

[fdintino]

As far as I can tell, you put Is in the correct place. I don't think the relative order of In and Is should matter; I'm having a hard time coming up with a scenario when the two could even compete.

[ArmorDarks]

I just wonder, is none is none and {% set none = 'Hey!' %} works same way in Jinja as null is null in this PR?

[ArmorDarks]

barely anything else works in IE 8 out-of-the-box anymore

Worth adding that IE8 here was more a figure of speech. Set and Symbols (and iterable protocol) are not available in much broader set of browsers, which includes not that old IE11

[noahlange]

According to CONTRIBUTING.md, Nunjucks should support IE8, so long as the ES5 shim is present. I mean, I'm not sure that that doesn't need updating, but it's not a figure of speech.

And re: nulls, I checked Jinja2. For the record:

• none is none is True.
• {% set none = 1 %} throws a syntax error.
• {% set null = 1 %} is okay.

[ArmorDarks]

According to CONTRIBUTING.md, Nunjucks should support IE8, so long as the ES5 shim is present. I mean, I'm not sure that that doesn't need updating, but it's not a figure of speech.

I meant that even if we drop IE8, there are still other browsers, which not much better than IE8 in some regards without babel or other polyfills. Sorry for confusion, my bad.

And re: nulls, I checked Jinja2. For the record:

Than personally I don't see anything against depreciating {% set null = 1 %} with this PR. It just mimics Jinja behavior, exactly what we want.