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

Nunjucks-slim fails on installJinjaCompat() #1019

Closed
krassowski opened this issue Aug 3, 2017 · 4 comments
Closed

Nunjucks-slim fails on installJinjaCompat() #1019

krassowski opened this issue Aug 3, 2017 · 4 comments
Labels

Comments

@krassowski
Copy link

Hi, many thanks for releasing the 3.0.1 version. I am trying to use the slice syntax with nunjucks-slim. I got help with pre-compilation in Jinja Compatibility mode, but I cannot use the pre-compiled tempates. It seems that there is a bug in installJinjaCompat() for nunjucks-slim; the bug:

nunjucks-slim.js:2910 Uncaught TypeError: Cannot read property 'prototype' of undefined
    at Object.installCompat [as installJinjaCompat] (nunjucks-slim.js:2910)
    at window.onload ((index):45)
installCompat @ nunjucks-slim.js:2910
window.onload @ (index):45

Following code is sufficient to reproduce:

<script src="https://cdnjs.cloudflare.com/ajax/libs/nunjucks/3.0.1/nunjucks-slim.js"></script>
<script>
nunjucks.installJinjaCompat();
</script>

And here is jsfiddle with some additional testing code.

Is it a bug, or an intended behaviour? How can I make Jinja compatibility mode work with nunjucks-slim?

krassowski added a commit to reimandlab/ActiveDriverDB that referenced this issue Aug 3, 2017
@noahlange
Copy link
Contributor

My guess is that jinja-compat is attempting to patch both the runtime and the compiler/parser, and since the latter are not included in slim, it's exploding.

var orig_Compiler_assertType = Compiler.prototype.assertType;
var orig_Parser_parseAggregate = Parser.prototype.parseAggregate;

My guess is that's where it's choking. Options include:

  1. split jinja-compat into runtime and compile-time components
  2. put some if/else blocks that prevent the compiler/parser patching from happening in slim

If this is still a relevant issue for y'all, I can take a look at writing a PR.

@fdintino
Copy link
Collaborator

Is there actually value in having a separate installJinjaCompat, rather than having it be merged in with standard functionality? It would certainly make the code simpler.

@noahlange
Copy link
Contributor

Hmm. I think if we were ever to consider Liquid or Twig compatibility patches, merging the changes from Jinja2's compatibility patch into "core" might turn out to bite us.

I really like the slice operator, but many of the other changes are very Pythonic.

@fdintino
Copy link
Collaborator

A fix for this is in the works.

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