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

Typescript class objects functions and getters return blank when used in tags #588

Closed fsuk closed 1 year ago

fsuk commented 1 year ago

Tags which access functions / getters on objects from typescript classes resolve as blank.

export class MyData {
  public get id () :string {
    return '12345'
  }
}
  const lq = new Liquid()
  const data = new MyData()
  const result = await lq.parseAndRender('The ID is: {{id}}', data)
  //The ID is:
fsuk commented 1 year ago

Workaround for functions:

export class MyData {
 getId = () => {
    return '12345'
  }
}
fsuk commented 1 year ago

Workaround for getters:

export class MyData {
  constructor () {
    Object.defineProperties(this, {
      id: {
        get: this._getId,
        enumerable: true
      }
    })
  }

  id: string
  private _getId (): string {
    return '12345'
  }
}

Note id: string so typescript knows it exists

harttle commented 1 year ago

Hey @fsuk ,

ES6 class getters are on __proto__, which is not allowed to read from by default, you'll need to set ownPropertyOnly to false to allow that. This should work for you:

    class MyData {
      public get id () :string {
        return '12345'
      }
    }
    const liquid = new Liquid({ ownPropertyOnly: false })
    const data = new MyData()
    const result = await liquid.parseAndRender('The ID is: {{id}}', data)
    expect(result).to.equal('The ID is: 12345')

For further discussion that may justify "reading from getters being an exception even ownPropertyOnly set to true", it can be implemented so in theory utilizing Object.getOwnPropertyDescriptor(), but that can be too heavy to check property descriptor each time reading a property. Since the implementation requires reading property descriptor of data.__proto__, I believe you should set ownPropertyOnly to false to explicitly tell LiquidJS that you're using Objects as context.

fsuk commented 1 year ago

According to the docs ownPropertyOnly defaults to false and I have not set it to true

harttle commented 1 year ago

Sorry the docs is outdated, the default value is changed to true since v10.0.0, I'm updating the docs.