liquid icon indicating copy to clipboard operation
liquid copied to clipboard

[StrainerTemplate] Isolate filter mods

Open tjoyal opened this issue 5 years ago • 1 comments

From #1208

This is a WIP proof of concept of an idea I have.

This is my simple and naive attempt at isolating filter modules from one another. This way private methods from one filter are not available by another.

Some items I still need to figure out:

  • Current logic calls new before invoking the filters. It could be limited to once per rendering. Not sure it's worth the engineering overhead, but performance first might mandate it.
  • Missing tests.

While this only include "sandboxing" in order to avoid filter collisions, I want to iterate later on with Liquid:Filter and experiment adding an extra concept of "declaring environment dependency" so that when calling tmpl.render we can pre-validate the context (most likely only in CI for perf reasons?) and detect missing Liquid::Registry from the Liquid::Context.

My long term goal is to have a smarter system that would detect inconsistencies in CI rather than waiting for a production execution to stumble on the error cases. Aim to avoiding "at runtime user facing exceptions" which require us to "wack a mole" as they show up.

Filters are defined once, not necessarily from your own codebase. In an app that is using multiple filters, it is not really reasonable for every code path that is calling render to test for every combinations imaginable, especially when templates are user controlled.

tjoyal avatar Jan 07 '20 15:01 tjoyal

I went back to initializing filters. This mean filters are initialized once each per content rendering. Uncertain of the perf tradeoff here, shouldn't be much?

If we really want to focus on perf, we could achieve larger things if we are to consider a bit of a paradigm change.

Filters as classes(added here) have the possibility to be completely stateless and moving away from modules would allow us to revisit the signatures.

Eg.: Not instantiating

class SomeFilter < Liquid::Filter
  def self.filter_name(context, input)
    ...
  end
end

But then revisiting open other doors. We could shift the responsibility of @context away from liquid and directly to the caller.

Traditionally one would set registries as an unstructured hash (which he need to know by reading the internals of each filters included). Then liquid would make these available to the filters.

By moving the responsibility around, code then can ensure required registry presence by adding it to the filter signature, making the code less error prone and easier to maintain.

I think this remove the need for context completely in filters.

class MagicSuffixFilter < Liquid::Filter
  def initialize(suffix)
    @suffix = suffix
  end

  def self.suffix(input)
    "#{input}_#{@suffix}"
  end
end

tmpl = Liquid::Template.parse('Result: {{ "test" | suffix }}')
filters = [
   MagicSuffixFilter.new("abcd")
]
tmpl.render({}, filters: filters)
=> "test_abcd"

tjoyal avatar Jan 22 '20 23:01 tjoyal