feature: context specific encoders

[noisesmith]

feature: context specific encoders

[dakrone]

Is there an obvious way to implement this with the existing cheshire design?

Not that I can immediately think of, I will have to experiment to see if there already is.

Is this a feature that would be welcome for cheshire if I implemented it?

Yes, definitely would be nice as long as it does not negatively affect performance.

[gfredericks]

This has been a big pain point for me as well.

One option I just thought of is to have another option that can be passed of a :fallback-encoder -- an encoder function that gets used whenever the JsonGenerationException would otherwise be thrown. Presumably that wouldn't be a big perf problem (at least for people not using it) since it's only a code change in an otherwise-exceptional codepath.

For my use cases I think the fallback encoder would pretty much always just use str, so we could also consider adding a feature that just does that, or at least an easier way to opt into that than an actual encoder fn.

[gfredericks]

Though now that I look through the implementation I remember that options are passed around as first-class args, and so having an extra option means expanding the signatures everywhere :/

[gfredericks]

If the signature thing is a big perf deal, maybe an internal dynamic var that gets set once at the top level would be an alternative.

[dakrone]

I remember that options are passed around as first-class args, and so having an extra option means expanding the signatures everywhere :/

I'd need to check the performance hit for it, but if it's not too much, a breaking change (Cheshire 6.x) to switch to a map of options would be better for future development.

[gfredericks]

I'll see if I can quickly make a branch with the options-map change.

[dakrone]

Okay, remember not to use destructuring as that (unfortunately) causes a big performance hit.

[gfredericks]

do you just mean destructuring when you don't actually need all the keys? When I macroexpand a {:keys [foo bar]} destructuring the only extra thing I see is a (seq? m) check, is that the bad part?

[dakrone]

I haven't dug into why it's slower yet, only that it was slower in benchmarks (heck, maybe Clojure has caught up and it's better in recent versions?), so it would be good to test and see.

[gfredericks]

okay; it ought to be identical to the macroexpanded code, but regardless I will leave it out so we don't have to wonder

[gfredericks]

Okay I have some code here that seems to work except for some bug in the generate-seq code that I don't have time to chase down at the moment. Hopefully the non-seq code is correct and adequate for perf tests.

[gfredericks]

(bug indicated by the one test failure)

[dakrone]

Okay I have some code here that seems to work

Great, thanks! No guarantees for when I will be able to look as my wife just had a baby and I am short on sleep/free-time, but I will try to take a look whenever I can! :)

[gfredericks]

babies babies babies babies babies babies babies babies babies babies babies babies!

[dakrone]

@gfredericks I finally had a second to check out your branch. Performance-wise it looks good to me, would you be able to open a PR for moving to it?

[gfredericks]

Will do

[gfredericks]

In terms of an API for extending via opts, the code used in greglook/puget#23 will probably be relevant.

[ikitommi]

Any news on this? Looking also forward to this, config similar like the edn / transit readers?