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

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

Closed
oscarotero opened this issue Dec 11, 2022 · 5 comments
Closed

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

oscarotero opened this issue Dec 11, 2022 · 5 comments

Comments

@oscarotero
Copy link

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 added a commit that referenced this issue Dec 12, 2022
github-actions bot pushed a commit that referenced this issue Dec 12, 2022
## [10.3.1](v10.3.0...v10.3.1) (2022-12-12)

### Bug Fixes

* support `Context` as `evalValue` parameter, [#568](#568) ([0f4916b](0f4916b))
@harttle
Copy link
Owner

harttle commented Dec 12, 2022

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
Copy link
Author

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
Copy link

🎉 This issue has been resolved in version 10.3.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@harttle
Copy link
Owner

harttle commented Dec 13, 2022

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

@oscarotero
Copy link
Author

Perfect. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants