harttle / liquidjs

A simple, expressive, safe and Shopify compatible template engine in pure JavaScript.
https://liquidjs.com
MIT License
1.52k stars 238 forks source link

Custom tag registration doesn't work in Liquid v10 #568

Closed oscarotero closed 1 year ago

oscarotero commented 1 year ago

Hi. In Liquidjs v9 I have the following code to create custom tags:

engine.registerTag('upper', {
  parse: function(tagToken, remainTokens) {
      this.str = tagToken.args;
  },
  render: async function(ctx) {
      const str = await this.liquid.evalValue(this.str, ctx);
      return str.toUpperCase();
  }
});

This code fails in Liquid 10: Uncaught RenderError: Cannot read properties of undefined (reading 'toUpperCase').

I found this example in the documentation but it doesn't work either:

engine.registerTag('upper', {
    parse: function(tagToken) {
        this.str = tagToken.args; // name
    },
    render: function*(ctx) {
        const str = yield this.liquid.evalValue(this.str, ctx);
        return str.toUpperCase()
    }
});
harttle commented 1 year ago

I didn't realize evalValue is advocated in docs and demos. I removed support for Context as second parameter when dealing with #527 because it's not used my source code. Now added it back because I think it's indeed convenient (so I used it in my docs).

Performance-wise, it's better to create new Value() during parse, and call value.value() in render(). As in the implementation of builtin tags.

oscarotero commented 1 year ago

Thanks for the fast response.

Performance-wise, it's better to create new Value() during parse, and call value.value() in render(). As in the implementation of builtin tags.

I don't have any preference, so if it's more perfomant this suggestion, I'll change it. Can you provide an example for that (or update the example in the documentation)? Somethig like this:

engine.registerTag('upper', {
  parse: function(tagToken, remainTokens) {
      this.val = new Value(tagToken.args);
  },
  render: async function(ctx) {
      const str = this.val.value();
      return str.toUpperCase();
  }
});
github-actions[bot] commented 1 year ago

:tada: This issue has been resolved in version 10.3.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

harttle commented 1 year ago

Updated this tutorial: https://liquidjs.com/tutorials/sync-and-async.html as well. Hope it helps!

oscarotero commented 1 year ago

Perfect. Thanks!