Repository: harttle/liquidjs
Pull request #705 (2024-06-04 20:02:30)

fix: use drop `valueOf` when evaluated as solitary condition
# Context

I noticed an inconsistency with drops where a drop's value will go through normal value resolution if used in an expression like `if drop > 10`, but it does not if the drop is the only part of the expression like `if drop` which causes it to always evaluate as truthy (because it's an object).

Shopify's Liquid implementation doesn't seem to support functionality similar to that of `valueOf` that we have in Liquid.js, so there's not prior art we can rely on there. Given if does seem to call `valueOf` when used with some type of operator I opted to make the bare drop case consistent with the existing behavior.


Patch:
diff --git a/src/render/boolean.spec.ts b/src/render/boolean.spec.ts
index 8dde9b18d..066e318a5 100644
--- a/src/render/boolean.spec.ts
+++ b/src/render/boolean.spec.ts
@@ -1,5 +1,6 @@
 import { isTruthy, isFalsy } from './boolean'
 import { Context } from '../context'
+import { Drop } from '..'
 
 describe('boolean Shopify', function () {
   describe('.isTruthy()', function () {
@@ -8,7 +9,13 @@ describe('boolean Shopify', function () {
         jsTruthy: false
       }
     } as unknown as Context
-    //
+
+    class BooleanDrop extends Drop {
+      public valueOf () {
+        return false
+      }
+    }
+
     // Spec: https://shopify.github.io/liquid/basics/truthy-and-falsy/
     it('true is truthy', function () {
       expect(isTruthy(true, ctx)).toBeTruthy()
@@ -40,6 +47,9 @@ describe('boolean Shopify', function () {
     it('[] is truthy', function () {
       expect(isTruthy([], ctx)).toBeTruthy()
     })
+    it('drop valueOf determines truthy', function () {
+      expect(isTruthy(new BooleanDrop(), ctx)).toBeFalsy()
+    })
   })
 })
 
diff --git a/src/render/boolean.ts b/src/render/boolean.ts
index d91c8f7f1..1ad08c2e1 100644
--- a/src/render/boolean.ts
+++ b/src/render/boolean.ts
@@ -1,10 +1,13 @@
 import { Context } from '../context/context'
+import { toValue } from '../util'
 
 export function isTruthy (val: any, ctx: Context): boolean {
   return !isFalsy(val, ctx)
 }
 
 export function isFalsy (val: any, ctx: Context): boolean {
+  val = toValue(val)
+
   if (ctx.opts.jsTruthy) {
     return !val
   } else {


Test patch:
diff --git a/test/integration/tags/if.spec.ts b/test/integration/tags/if.spec.ts
index 2086b0360..5dd25850c 100644
--- a/test/integration/tags/if.spec.ts
+++ b/test/integration/tags/if.spec.ts
@@ -1,4 +1,4 @@
-import { Liquid } from '../../../src/liquid'
+import { Liquid, Drop } from '../../../src'
 
 describe('tags/if', function () {
   const liquid = new Liquid()
@@ -9,6 +9,12 @@ describe('tags/if', function () {
     emptyArray: []
   }
 
+  class BooleanDrop extends Drop {
+    public valueOf () {
+      return false
+    }
+  }
+
   it('should throw if not closed', function () {
     const src = '{% if false%}yes'
     return expect(liquid.parseAndRender(src, scope))
@@ -143,6 +149,12 @@ describe('tags/if', function () {
     const html = await liquid.parseAndRender(src, scope)
     return expect(html).toBe('success')
   })
+  it('should support drop as condition variable', async () => {
+    const src = `{% if drop %}yes{% else %}no{% endif %}`
+    const scope = { drop: new BooleanDrop() }
+    const html = await liquid.parseAndRender(src, scope)
+    return expect(html).toBe('no')
+  })
   it('should not render anything after an else branch even when first else branch is empty', () => {
     const engine = new Liquid()
     const result = engine.parseAndRenderSync('{% if false %}don\'t show' +