Avoid hash with default values due to inconsistent marshaling

[maximebedard]

Encoding/Decoding the liquid context using different encoders doesn't always encode hashes and remember that they can have a default value.

# Poor example, but `snappy` would behave the same way.
irb(main):009:0> JSON.load(JSON.dump(Hash.new(0)))[0]
=> nil # => 0 would have been expected

Instead, it's easier to just avoid using default values here and simply cast them to Fixnum where needed.

[fw42]

I don't understand the context of this patch. What problem does this solve? This seems a bit like a workaround for a bug in some other piece (that has nothing to do with Liquid).

[dylanahsmith]

What is the use case for serializing and deserializing the context? I know we experimented with serializing and deserializing parsed templates to avoid parsing overhead, but I don't see why we would care about that for the context.

If we were going to care about the context being serializable, then we should have a test for that.

[pushrax]

👍 to this change but I'd also like to hear more about why you need this.

[dylanahsmith]

He explained the context out of band. Basically this is needed for fragment caching, since we would need to cache register changes and apply them when using the cached fragment.

[maximebedard]

Sorry getting back to this after a little while. Would it make sense to ship this PR as it currently is and then build integration tests that make sure the Liquid context is fully serializable/deserializable or simple unit test that assert the hash has no defaults would suffice?

[dylanahsmith]

build integration tests that make sure the Liquid context is fully serializable/deserializable or simple unit test that assert the hash has no defaults would suffice?

Basically we want a test that avoids a regression in whatever we are doing on top of liquid. So if that code is serializing and deserializing the whole context, then that would be more appropriate to do in a test to avoid that regression. Having a test that checks that the register value to make sure it is a hash without defaults overly coupled to the implementation and wouldn't test as much, so it would be easier to introduce a regression somewhere else in the context.

Would it make sense to ship this PR as it currently is and then

Sure, feel free to to unblock your work.