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

Add support for strict type check comparisons #746

Merged
merged 2 commits into from
May 6, 2016

Conversation

gt53
Copy link
Contributor

@gt53 gt53 commented May 5, 2016

I added support for the === and !== comparison operators.

@carljm
Copy link
Contributor

carljm commented May 6, 2016

This is a departure from Jinja2 syntax, which normally I would prefer not to do, but in this case I buy that it's necessary; == already behaves differently in nunjucks vs Jinja2 because internally it's a Python == vs a JS ==, and those are different. I don't believe this is a difference we will reasonably be able to paper over. And if you're limited to JS == semantics, then you need the option of === as well.

@carljm
Copy link
Contributor

carljm commented May 6, 2016

If I were implementing nunjucks from scratch, I would be awfully tempted to have == in a nunjucks template always translate to === at the JS level, but I think that ship has sailed; it would be a seriously backwards-incompatible change.

@carljm carljm merged commit 0331bf1 into mozilla:master May 6, 2016
@ricordisamoa
Copy link
Contributor

Why not in a new major version?

@carljm
Copy link
Contributor

carljm commented May 6, 2016

@ricordisamoa I'm open to that, if you want to open a ticket for it and put it in the 3.0 milestone.

Hmm... I guess if we plan to do that, it might be better not to merge this? It would be silly to end up in a situation where we support both == and ===, but both actually mean ===.

On the other hand, if it's in 3.0 we can always just drop === at that point again...

carljm added a commit that referenced this pull request May 6, 2016
Add support for strict type check comparisons
@gt53
Copy link
Contributor Author

gt53 commented May 6, 2016

@carljm, thanks for going over my changes. I understand the Jinja2 influence and desire not to stray from that. I hope that a benefit from this new functionality is a lower barrier to adoption for swig users looking to migrate their templates since swig is no longer maintained. Nunjucks seems like the natural choice for migrating swig templates, and swig has strict type checking.

I’m in the process of migrating some swig templates to nunjucks and ran into === not compiling for nunjucks. It’s not a big deal to replace === with ==, but some time needs to be spent making sure that the comparison doesn’t actually need to be type strict, or if === was used because developers have been beaten over the head by linters to use that whenever they write JS (and writing templates can feel like writing regular JS).

@ricordisamoa
Copy link
Contributor

I'm open to that, if you want to open a ticket for it and put it in the 3.0 milestone.

#750

This was referenced May 26, 2016
carljm pushed a commit that referenced this pull request May 26, 2016
It should have been #746 instead of #710
carljm pushed a commit that referenced this pull request May 27, 2016
It should have been #746 instead of #710
@ArmorDarks
Copy link

There is a typo in release notes:

Add support for strict type check comparisons (=== and !===). Thanks

!=== should be !==

I also hope that #750 is still valid for 3.0.0 version?

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

Successfully merging this pull request may close these issues.

None yet

4 participants