From 6e6ea0ae588549d1c2693cc3a8831de6973ff4ac Mon Sep 17 00:00:00 2001 From: harttle Date: Mon, 22 Jul 2019 16:07:24 +0800 Subject: [PATCH] fix: some filters on undefined variable throws, #140 --- README.md | 2 +- src/builtin/filters/array.ts | 56 +++++++++----- src/builtin/filters/string.ts | 75 +++++++++++------- src/util/underscore.ts | 7 +- test/integration/builtin/filters/array.ts | 90 +++++++++++++++++++--- test/integration/builtin/filters/math.ts | 4 + test/integration/builtin/filters/string.ts | 70 ++++++++++++++--- test/unit/util/underscore.ts | 6 -- 8 files changed, 229 insertions(+), 81 deletions(-) diff --git a/README.md b/README.md index 9f2853a7db..361e7c4953 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ though there are still some differences: * Dynamic file locating (enabled by default), that means layout/partial names are treated as variables in liquidjs. See [#51](https://github.com/harttle/liquidjs/issues/51). * Truthy and Falsy. All values except `undefined`, `null`, `false` are truthy, whereas in Ruby Liquid all except `nil` and `false` are truthy. See [#26](https://github.com/harttle/liquidjs/pull/26). -* Number Rendering. Since JavaScript do not distinguish `float` and `integer`, we cannot either convert between them nor render regarding to their type. See [#59](https://github.com/harttle/liquidjs/issues/59). +* Number. In JavaScript we cannot distinguish or convert between `float` and `integer`, see [#59](https://github.com/harttle/liquidjs/issues/59). And when applied `size` filter, numbers always return 0, which is 8 for integer in ruby, cause they do not have a `length` property. * [.to_liquid()](https://github.com/Shopify/liquid/wiki/Introduction-to-Drops) is replaced by `.toLiquid()` * [.to_s()](https://www.rubydoc.info/gems/liquid/Liquid/Drop) is replaced by JavaScript `.toString()` diff --git a/src/builtin/filters/array.ts b/src/builtin/filters/array.ts index d357abe04c..24bba707f4 100644 --- a/src/builtin/filters/array.ts +++ b/src/builtin/filters/array.ts @@ -1,28 +1,42 @@ -import { last } from '../../util/underscore' +import { isArray, last } from '../../util/underscore' import { isTruthy } from '../../render/syntax' export default { 'join': (v: any[], arg: string) => v.join(arg === undefined ? ' ' : arg), - 'last': (v: T[]): T => last(v), - 'first': (v: T[]): T => v[0], - 'map': (arr: {[key: string]: T1}[], arg: string): T1[] => arr.map(v => v[arg]), + 'last': (v: any) => isArray(v) ? last(v) : '', + 'first': (v: any) => isArray(v) ? v[0] : '', + 'map': map, 'reverse': (v: any[]) => [...v].reverse(), 'sort': (v: T[], arg: (lhs: T, rhs: T) => number) => v.sort(arg), - 'size': (v: string | any[]) => v.length, - 'concat': (v: T1[], arg: T2[] | T2): (T1 | T2)[] => Array.prototype.concat.call(v, arg), - 'slice': (v: T[], begin: number, length: number = 1): T[] => { - begin = begin < 0 ? v.length + begin : begin - return v.slice(begin, begin + length) - }, - 'uniq': function (arr: T[]): T[] { - const u = {} - return (arr || []).filter(val => { - if (u.hasOwnProperty(String(val))) return false - u[String(val)] = true - return true - }) - }, - 'where': function (arr: T[], property: string, value?: any): T[] { - return arr.filter(obj => value === undefined ? isTruthy(obj[property]) : obj[property] === value) - } + 'size': (v: string | any[]) => (v && v.length) || 0, + 'concat': concat, + 'slice': slice, + 'uniq': uniq, + 'where': where +} + +function map (arr: {[key: string]: T1}[], arg: string): T1[] { + return arr.map(v => v[arg]) +} + +function concat (v: T1[], arg: T2[] | T2): (T1 | T2)[] { + return Array.prototype.concat.call(v, arg) +} + +function slice (v: T[], begin: number, length: number = 1): T[] { + begin = begin < 0 ? v.length + begin : begin + return v.slice(begin, begin + length) +} + +function where (arr: T[], property: string, value?: any): T[] { + return arr.filter(obj => value === undefined ? isTruthy(obj[property]) : obj[property] === value) +} + +function uniq (arr: T[]): T[] { + const u = {} + return (arr || []).filter(val => { + if (u.hasOwnProperty(String(val))) return false + u[String(val)] = true + return true + }) } diff --git a/src/builtin/filters/string.ts b/src/builtin/filters/string.ts index bdc11d7180..b868f3f044 100644 --- a/src/builtin/filters/string.ts +++ b/src/builtin/filters/string.ts @@ -1,28 +1,51 @@ +/** + * String related filters + * + * * prefer stringify() to String() since `undefined`, `null` should eval '' + */ +import { stringify } from '../../util/underscore' + export default { - 'append': (v: string, arg: string) => v + arg, - 'prepend': (v: string, arg: string) => arg + v, - 'capitalize': (str: string) => String(str).charAt(0).toUpperCase() + str.slice(1), - 'lstrip': (v: string) => String(v).replace(/^\s+/, ''), - 'downcase': (v: string) => v.toLowerCase(), - 'upcase': (str: string) => String(str).toUpperCase(), - 'remove': (v: string, arg: string) => v.split(arg).join(''), - 'remove_first': (v: string, l: string) => v.replace(l, ''), - 'replace': (v: string, pattern: string, replacement: string) => - String(v).split(pattern).join(replacement), - 'replace_first': (v: string, arg1: string, arg2: string) => String(v).replace(arg1, arg2), - 'rstrip': (str: string) => String(str).replace(/\s+$/, ''), - 'split': (v: string, arg: string) => String(v).split(arg), - 'strip': (v: string) => String(v).trim(), - 'strip_newlines': (v: string) => String(v).replace(/\n/g, ''), - 'truncate': (v: string, l: number = 50, o: string = '...') => { - v = String(v) - if (v.length <= l) return v - return v.substr(0, l - o.length) + o - }, - 'truncatewords': (v: string, l: number = 15, o: string = '...') => { - const arr = v.split(/\s+/) - let ret = arr.slice(0, l).join(' ') - if (arr.length >= l) ret += o - return ret - } + 'append': (v: string, arg: string) => stringify(v) + arg, + 'prepend': (v: string, arg: string) => arg + stringify(v), + 'capitalize': capitalize, + 'lstrip': (v: string) => stringify(v).replace(/^\s+/, ''), + 'downcase': (v: string) => stringify(v).toLowerCase(), + 'upcase': (str: string) => stringify(str).toUpperCase(), + 'remove': (v: string, arg: string) => stringify(v).split(arg).join(''), + 'remove_first': (v: string, l: string) => stringify(v).replace(l, ''), + 'replace': replace, + 'replace_first': replaceFirst, + 'rstrip': (str: string) => stringify(str).replace(/\s+$/, ''), + 'split': (v: string, arg: string) => stringify(v).split(arg), + 'strip': (v: string) => stringify(v).trim(), + 'strip_newlines': (v: string) => stringify(v).replace(/\n/g, ''), + 'truncate': truncate, + 'truncatewords': truncateWords +} + +function capitalize (str: string) { + str = stringify(str) + return str.charAt(0).toUpperCase() + str.slice(1) +} + +function replace (v: string, pattern: string, replacement: string) { + return stringify(v).split(pattern).join(replacement) +} + +function replaceFirst (v: string, arg1: string, arg2: string) { + return stringify(v).replace(arg1, arg2) +} + +function truncate (v: string, l: number = 50, o: string = '...') { + v = stringify(v) + if (v.length <= l) return v + return v.substr(0, l - o.length) + o +} + +function truncateWords (v: string, l: number = 15, o: string = '...') { + const arr = v.split(/\s+/) + let ret = arr.slice(0, l).join(' ') + if (arr.length >= l) ret += o + return ret } diff --git a/src/util/underscore.ts b/src/util/underscore.ts index 986f6d88b4..76b2cc34e9 100644 --- a/src/util/underscore.ts +++ b/src/util/underscore.ts @@ -28,9 +28,8 @@ export function promisify (fn: any) { } export function stringify (value: any): string { - if (isNil(value)) return '' - value = toLiquid(value) - return String(value) + value = toValue(value) + return isNil(value) ? '' : String(value) } export function toValue (value: any): any { @@ -42,7 +41,7 @@ export function isNumber (value: any): value is number { } export function toLiquid (value: any): any { - if (isFunction(value.toLiquid)) return toLiquid(value.toLiquid()) + if (value && isFunction(value.toLiquid)) return toLiquid(value.toLiquid()) return value } diff --git a/test/integration/builtin/filters/array.ts b/test/integration/builtin/filters/array.ts index 2560341a3e..d67abff4ae 100644 --- a/test/integration/builtin/filters/array.ts +++ b/test/integration/builtin/filters/array.ts @@ -40,17 +40,85 @@ describe('filters/array', function () { }) }) describe('size', function () { - it('should return string length', - () => test('{{ "Ground control to Major Tom." | size }}', '28')) - it('should return array size', function () { - return test('{% assign my_array = "apples, oranges, peaches, plums"' + - ' | split: ", " %}{{ my_array | size }}', - '4') - }) - it('should be respected with .size notation', - () => test('{% assign my_string = "Ground control to Major Tom." %}{{ my_string.size }}', '28')) - it('should be respected with .size notation', - () => test('{% assign my_array = "apples, oranges, peaches, plums" | split: ", " %}{{ my_array.size }}', '4')) + it('should return string length', async () => { + const html = await liquid.parseAndRender('{{ "Ground control to Major Tom." | size }}') + expect(html).to.equal('28') + }) + it('should return array size', async () => { + const html = await liquid.parseAndRender( + '{% assign my_array = "apples, oranges, peaches, plums" | split: ", " %}{{ my_array | size }}') + expect(html).to.equal('4') + }) + it('should be respected with .size notation', async () => { + const html = await liquid.parseAndRender('{% assign my_string = "Ground control to Major Tom." %}{{ my_string.size }}') + expect(html).to.equal('28') + }) + it('should be respected with .size notation', async () => { + const html = await liquid.parseAndRender('{% assign my_array = "apples, oranges, peaches, plums" | split: ", " %}{{ my_array.size }}') + expect(html).to.equal('4') + }) + it('should return 0 for false', async () => { + const html = await liquid.parseAndRender('{{ false | size }}') + expect(html).to.equal('0') + }) + it('should return 0 for nil', async () => { + const html = await liquid.parseAndRender('{{ nil | size }}') + expect(html).to.equal('0') + }) + it('should return 0 for undefined', async () => { + const html = await liquid.parseAndRender('{{ foo | size }}') + expect(html).to.equal('0') + }) + }) + describe('first', function () { + it('should support first', async () => { + const html = await liquid.parseAndRender( + '{{arr | first}}', + { arr: [ 'zebra', 'tiger' ] } + ) + expect(html).to.equal('zebra') + }) + it('should return empty for nil', async () => { + const html = await liquid.parseAndRender('{{nil | first}}') + expect(html).to.equal('') + }) + it('should return empty for undefined', async () => { + const html = await liquid.parseAndRender('{{foo | first}}') + expect(html).to.equal('') + }) + it('should return empty for false', async () => { + const html = await liquid.parseAndRender('{{false | first}}') + expect(html).to.equal('') + }) + it('should return empty for string', async () => { + const html = await liquid.parseAndRender('{{"zebra" | first}}') + expect(html).to.equal('') + }) + }) + describe('last', function () { + it('should support last', async () => { + const html = await liquid.parseAndRender( + '{{arr | last}}', + { arr: [ 'zebra', 'tiger' ] } + ) + expect(html).to.equal('tiger') + }) + it('should return empty for nil', async () => { + const html = await liquid.parseAndRender('{{nil | last}}') + expect(html).to.equal('') + }) + it('should return empty for undefined', async () => { + const html = await liquid.parseAndRender('{{foo | last}}') + expect(html).to.equal('') + }) + it('should return empty for false', async () => { + const html = await liquid.parseAndRender('{{false | last}}') + expect(html).to.equal('') + }) + it('should return empty for string', async () => { + const html = await liquid.parseAndRender('{{"zebra" | last}}') + expect(html).to.equal('') + }) }) describe('slice', function () { it('should slice first char by 0', () => test('{{ "Liquid" | slice: 0 }}', 'L')) diff --git a/test/integration/builtin/filters/math.ts b/test/integration/builtin/filters/math.ts index f929101121..823e367023 100644 --- a/test/integration/builtin/filters/math.ts +++ b/test/integration/builtin/filters/math.ts @@ -60,6 +60,10 @@ describe('filters/math', function () { () => test('{{ 183.357 | plus: 12 }}', '195.357')) it('should convert first arg as number', () => test('{{ "4" | plus: 2 }}', '6')) it('should convert both args as number', () => test('{{ "4" | plus: "2" }}', '6')) + it('should support variable', async () => { + const html = await l.parseAndRender('{{ 4 | plus: b }}', { b: 2 }) + expect(html).to.equal('6') + }) }) describe('sort_natural', function () { diff --git a/test/integration/builtin/filters/string.ts b/test/integration/builtin/filters/string.ts index 0839835d14..2aa423145b 100644 --- a/test/integration/builtin/filters/string.ts +++ b/test/integration/builtin/filters/string.ts @@ -1,13 +1,30 @@ import { test } from '../../../stub/render' +import Liquid from '../../../../src/liquid' +import { expect } from 'chai' describe('filters/string', function () { + let liquid: Liquid + beforeEach(function () { + liquid = new Liquid() + }) describe('append', function () { it('should return "-3abc" for -3, "abc"', () => test('{{ -3 | append: "abc" }}', '-3abc')) it('should return "abar" for "a",foo', () => test('{{ "a" | append: foo }}', 'abar')) }) describe('capitalize', function () { - it('should capitalize first', () => test('{{ "i am good" | capitalize }}', 'I am good')) + it('should capitalize first', async () => { + const html = await liquid.parseAndRender('{{ "i am good" | capitalize }}') + expect(html).to.equal('I am good') + }) + it('should return empty for nil', async () => { + const html = await liquid.parseAndRender('{{ nil | capitalize }}') + expect(html).to.equal('') + }) + it('should return empty for undefined', async () => { + const html = await liquid.parseAndRender('{{ foo | capitalize }}') + expect(html).to.equal('') + }) }) describe('concat', function () { it('should concat arrays', () => test(` @@ -45,10 +62,18 @@ describe('filters/string', function () { `)) }) describe('downcase', function () { - it('should return "parker moore" for "Parker Moore"', - () => test('{{ "Parker Moore" | downcase }}', 'parker moore')) - it('should return "apple" for "apple"', - () => test('{{ "apple" | downcase }}', 'apple')) + it('should return "parker moore" for "Parker Moore"', async () => { + const html = await liquid.parseAndRender('{{ "Parker Moore" | downcase }}') + expect(html).to.equal('parker moore') + }) + it('should return "apple" for "apple"', async () => { + const html = await liquid.parseAndRender('{{ "apple" | downcase }}') + expect(html).to.equal('apple') + }) + it('should return empty for undefined', async () => { + const html = await liquid.parseAndRender('{{ foo | downcase }}') + expect(html).to.equal('') + }) }) describe('split', function () { it('should support split/first', function () { @@ -57,7 +82,16 @@ describe('filters/string', function () { return test(src, 'apples') }) }) - it('should support upcase', () => test('{{ "Parker Moore" | upcase }}', 'PARKER MOORE')) + describe('upcase', function () { + it('should support upcase', async () => { + const html = await liquid.parseAndRender('{{ "Parker Moore" | upcase }}') + expect(html).to.equal('PARKER MOORE') + }) + it('should return empty for undefined', async () => { + const html = await liquid.parseAndRender('{{ foo | upcase }}') + expect(html).to.equal('') + }) + }) it('should support lstrip', function () { const src = '{{ " So much room for activities! " | lstrip }}' return test(src, 'So much room for activities! ') @@ -78,13 +112,25 @@ describe('filters/string', function () { '{{ "/index.html" | prepend: url }}', 'liquidmarkup.com/index.html') }) - it('should support remove', function () { - return test('{{ "I strained to see the train through the rain" | remove: "rain" }}', - 'I sted to see the t through the ') + describe('remove', function () { + it('should support remove', async () => { + const html = await liquid.parseAndRender('{{ "I strained to see the train through the rain" | remove: "rain" }}') + expect(html).to.equal('I sted to see the t through the ') + }) + it('should return empty for undefined', async () => { + const html = await liquid.parseAndRender('{{ foo | remove: "rain" }}') + expect(html).to.equal('') + }) }) - it('should support remove_first', function () { - return test('{{ "I strained to see the train through the rain" | remove_first: "rain" }}', - 'I sted to see the train through the rain') + describe('remove_first', function () { + it('should support remove_first', async () => { + const html = await liquid.parseAndRender('{{ "I strained to see the train through the rain" | remove_first: "rain" }}') + expect(html).to.equal('I sted to see the train through the rain') + }) + it('should return empty for undefined', async () => { + const html = await liquid.parseAndRender('{{ foo | remove_first: "r" }}') + expect(html).to.equal('') + }) }) it('should support replace', function () { return test('{{ "Take my protein pills and put my helmet on" | replace: "my", "your" }}', diff --git a/test/unit/util/underscore.ts b/test/unit/util/underscore.ts index 5b08eb7e4f..70e092de7e 100644 --- a/test/unit/util/underscore.ts +++ b/test/unit/util/underscore.ts @@ -27,12 +27,6 @@ describe('util/underscore', function () { }) }) describe('.stringify()', function () { - it('should respect to toLiquid() method', function () { - expect(_.stringify({ toLiquid: () => 'foo' })).to.equal('foo') - }) - it('should recursively call toLiquid()', function () { - expect(_.stringify({ toLiquid: () => ({ toLiquid: () => 'foo' }) })).to.equal('foo') - }) it('should return "" for null', function () { expect(_.stringify(null)).to.equal('') })