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

A few built-in filters don't support undefined values #481

Closed
pdehaan opened this issue Feb 24, 2022 · 2 comments
Closed

A few built-in filters don't support undefined values #481

pdehaan opened this issue Feb 24, 2022 · 2 comments
Labels

Comments

@pdehaan
Copy link
Contributor

pdehaan commented Feb 24, 2022

Related to #479
liquidjs@9.35.0

ERROR IN {{ foo | join }}: Cannot read properties of undefined (reading 'join'), line:1, col:1
ERROR IN {{ foo | map }}: Cannot read properties of undefined (reading 'split'), line:1, col:1
ERROR IN {{ foo | reverse }}: undefined is not iterable (cannot read property Symbol(Symbol.iterator)), line:1, col:1
ERROR IN {{ foo | slice }}: Cannot read properties of undefined (reading 'slice'), line:1, col:1
ERROR IN {{ foo | newline_to_br }}: Cannot read properties of undefined (reading 'replace'), line:1, col:1
ERROR IN {{ foo | strip_html }}: Cannot read properties of undefined (reading 'replace'), line:1, col:1
ERROR IN {{ foo | truncatewords }}: Cannot read properties of undefined (reading 'split'), line:1, col:1

Apologies for the bad code:

// liquid-filter-test.mjs
import {Liquid} from "liquidjs";

const APPEND_JSON_FILTER = false;

function createFilterTest(name) {
  // Filters that need a second parameter...
  const secondParam = (["append", "prepend"].includes(name) ? ": bar" : "");
  // Don't double append edge case "|json |json"...
  const jsonFilter = name !== "json" && !!APPEND_JSON_FILTER ? " | json" : "";
  return `{{ foo | ${name}${secondParam}${jsonFilter} }}`;
}

const arrayFilters = ["concat", "first", "join", "json", "last", "map", "reverse", "slice", "sort_natural", "sort", "uniq", "where"];
const dateFilters = ["date"];
const htmlUriFilters = ["escape", "escape_once", "newline_to_br", "strip_html", "url_decode", "url_encode"];
const mathFilters = ["abs", "ceil", "divided_by", "floor", "minus", "modulo", "plus", "round", "times"];
const stringFilters = ["append", "capitalize", "downcase", "lstrip", "prepend", "remove", "remove_first", "replace", "replace_first", "rstrip", "split", "strip", "strip_newlines", "truncate", "truncatewords", "upcase"];

const allFilters = [
  ...arrayFilters,
  ...dateFilters,
  ...htmlUriFilters,
  ...mathFilters,
  ...stringFilters,
].map(name => createFilterTest(name));

const engine = new Liquid();
const results = [];

for (const input of allFilters) {
  const tpl = engine.parse(input);
  try {
    const res = await engine.render(tpl, {foo: undefined, bar: undefined})
    results.push(`${input} = ${res}`);
  } catch (err) {
    console.error(`ERROR IN ${input}: ${err.message}`);
  }
}

console.log(results.sort().join("\n"));

OUTPUT

{{ foo | abs }} = NaN
{{ foo | append: bar }} = 
{{ foo | capitalize }} = 
{{ foo | ceil }} = NaN
{{ foo | concat }} = 
{{ foo | date }} = 
{{ foo | divided_by }} = NaN
{{ foo | downcase }} = 
{{ foo | escape }} = 
{{ foo | escape_once }} = undefined
{{ foo | first }} = 
{{ foo | floor }} = NaN
{{ foo | json }} = 
{{ foo | last }} = 
{{ foo | lstrip }} = 
{{ foo | minus }} = NaN
{{ foo | modulo }} = NaN
{{ foo | plus }} = NaN
{{ foo | prepend: bar }} = 
{{ foo | remove }} = 
{{ foo | remove_first }} = 
{{ foo | replace }} = 
{{ foo | replace_first }} = 
{{ foo | round }} = NaN
{{ foo | rstrip }} = 
{{ foo | sort }} = 
{{ foo | sort_natural }} = 
{{ foo | split }} = 
{{ foo | strip }} = 
{{ foo | strip_newlines }} = 
{{ foo | times }} = NaN
{{ foo | truncate }} = 
{{ foo | uniq }} = 
{{ foo | upcase }} = 
{{ foo | url_decode }} = 
{{ foo | url_encode }} = 
{{ foo | where }} =

Interestingly it looks like {{ undefined | escape_once }} returns the literal string "undefined". Otherwise nothing too suspicious looking here (all the math functions seems to return NaN, which is semi-expected).


If I append the | json filter to each test case, it gets more interesting:

{{ foo | abs | json }} = null
{{ foo | append: bar | json }} = ""
{{ foo | capitalize | json }} = ""
{{ foo | ceil | json }} = null
{{ foo | concat | json }} = [null,null]
{{ foo | date | json }} = 
{{ foo | divided_by | json }} = null
{{ foo | downcase | json }} = ""
{{ foo | escape | json }} = ""
{{ foo | escape_once | json }} = "undefined"
{{ foo | first | json }} = ""
{{ foo | floor | json }} = null
{{ foo | json }} = 
{{ foo | last | json }} = ""
{{ foo | lstrip | json }} = ""
{{ foo | minus | json }} = null
{{ foo | modulo | json }} = null
{{ foo | plus | json }} = null
{{ foo | prepend: bar | json }} = ""
{{ foo | remove | json }} = ""
{{ foo | remove_first | json }} = ""
{{ foo | replace | json }} = ""
{{ foo | replace_first | json }} = ""
{{ foo | round | json }} = null
{{ foo | rstrip | json }} = ""
{{ foo | sort | json }} = [null]
{{ foo | sort_natural | json }} = []
{{ foo | split | json }} = [""]
{{ foo | strip | json }} = ""
{{ foo | strip_newlines | json }} = ""
{{ foo | times | json }} = null
{{ foo | truncate | json }} = ""
{{ foo | uniq | json }} = []
{{ foo | upcase | json }} = ""
{{ foo | url_decode | json }} = ""
{{ foo | url_encode | json }} = ""
{{ foo | where | json }} = []

It looks like the NaN now converts to null (presumably NaN isn't valid JSON). Interesting cases might be:

{{ undefined | concat | json }} = [null,null]
{{ undefined | escape_once | json }} = "undefined"
{{ undefined | sort | json }} = [null]
{{ undefined | sort_natural | json }} = []
{{ undefined | split | json }} = [""]
@harttle
Copy link
Owner

harttle commented Feb 26, 2022

I checked the behavior of shopify/liquid for these cases (except that they don't have a json filter). Here's some changes will be made on LiquidJS to align with Ruby:

  • filters like join, reverse, truncatewords wont throw on nil input, LiquidJS will not throw as well
  • concat, sort, sort_natural will ignore nil value input so undefined will not be added into result array. Actually, toArray will return [] for null/undefined instead of [null]/[undefined]
  • split will return empty array for nil input and remove trailing empty strings

@harttle harttle added the bug label Feb 26, 2022
github-actions bot pushed a commit that referenced this issue Feb 26, 2022
## [9.35.1](v9.35.0...v9.35.1) (2022-02-26)

### Bug Fixes

* some filters throw on nil input, see [#481](#481) ([7dfb620](7dfb620))
@pdehaan
Copy link
Contributor Author

pdehaan commented Feb 27, 2022

Awesome, thanks… Here's the current results for 9.35.1 (where foo is undefined):

FILTER VALUE +JSON FILTER VALUE
{{ foo | abs }} NaN {{ foo | abs | json }} null
{{ foo | append: bar }} {{ foo | append: bar | json }} ""
{{ foo | capitalize }} {{ foo | capitalize | json }} ""
{{ foo | ceil }} NaN {{ foo | ceil | json }} null
{{ foo | concat }} {{ foo | concat | json }} [null]
{{ foo | date }} {{ foo | date | json }}
{{ foo | divided_by }} NaN {{ foo | divided_by | json }} null
{{ foo | downcase }} {{ foo | downcase | json }} ""
{{ foo | escape }} {{ foo | escape | json }} ""
{{ foo | escape_once }} {{ foo | escape_once | json }} ""
{{ foo | first }} {{ foo | first | json }} ""
{{ foo | floor }} NaN {{ foo | floor | json }} null
{{ foo | join }} {{ foo | join | json }} ""
{{ foo | json }} {{ foo | json }}
{{ foo | last }} {{ foo | last | json }} ""
{{ foo | lstrip }} {{ foo | lstrip | json }} ""
{{ foo | map }} {{ foo | map | json }} []
{{ foo | minus }} NaN {{ foo | minus | json }} null
{{ foo | modulo }} NaN {{ foo | modulo | json }} null
{{ foo | newline_to_br }} {{ foo | newline_to_br | json }} ""
{{ foo | plus }} NaN {{ foo | plus | json }} null
{{ foo | prepend: bar }} {{ foo | prepend: bar | json }} ""
{{ foo | remove }} {{ foo | remove | json }} ""
{{ foo | remove_first }} {{ foo | remove_first | json }} ""
{{ foo | replace }} {{ foo | replace | json }} ""
{{ foo | replace_first }} {{ foo | replace_first | json }} ""
{{ foo | reverse }} {{ foo | reverse | json }} []
{{ foo | round }} NaN {{ foo | round | json }} null
{{ foo | rstrip }} {{ foo | rstrip | json }} ""
{{ foo | slice }} {{ foo | slice | json }} []
{{ foo | sort }} {{ foo | sort | json }} []
{{ foo | sort_natural }} {{ foo | sort_natural | json }} []
{{ foo | split }} {{ foo | split | json }} []
{{ foo | strip }} {{ foo | strip | json }} ""
{{ foo | strip_html }} {{ foo | strip_html | json }} ""
{{ foo | strip_newlines }} {{ foo | strip_newlines | json }} ""
{{ foo | times }} NaN {{ foo | times | json }} null
{{ foo | truncate }} {{ foo | truncate | json }} ""
{{ foo | truncatewords }} {{ foo | truncatewords | json }} ""
{{ foo | uniq }} {{ foo | uniq | json }} []
{{ foo | upcase }} {{ foo | upcase | json }} ""
{{ foo | url_decode }} {{ foo | url_decode | json }} ""
{{ foo | url_encode }} {{ foo | url_encode | json }} ""
{{ foo | where }} {{ foo | where | json }} []

Looks like the only curious entry in 9.35.1 is {{ undefined | concat }} which returns [null]:

<p>{{ foo | concat | size }} === 1</p>
<p>{{ foo | concat | json }} === [null]</p>

Liquid Playground example

github-actions bot pushed a commit that referenced this issue Mar 2, 2022
## [9.35.2](v9.35.1...v9.35.2) (2022-03-02)

### Bug Fixes

* corner case for concat filter without argument, [#481](#481) ([aa95517](aa95517))
* export all builtin tags from LiquidJS, [#464](#464) ([33009bb](33009bb))
@harttle harttle closed this as completed Mar 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants