Flag to optionally disable transit analysis cache encoding
Description
Environment
Attachments
Activity
David Nolen November 8, 2016 at 6:26 PM
António Monteiro November 8, 2016 at 1:44 PM
Attached CLJS-1666-1.patch which takes into account feedback obtained in Slack.
Specifically:
added validation for the `:cache-analysis-format` option
explicit inference of the cache file format via the new function `cache-analysis-ext`
Thomas Heller November 8, 2016 at 10:13 AM
As per Slack discussion I opened a separate issue to focus on the actual problem.
See: http://dev.clojure.org/jira/browse/CLJS-1843
I still think however that the newly introduced :cache-analysis-format is not useful to the user. If Transit is available it should always be used as it is the "better" option. I'm not sure there is a benefit of letting the user decide other than maybe work around issues that Transit has that EDN might not have. Hiding the actual problem by doing so.
If the cache write "fails" for any reason it should just warn about it instead of failing the build.
Thomas Heller November 7, 2016 at 9:53 PM
Will the toggle to :edn not just "hide" a problem?
Say the user somehow manages to break the transit encoding by using custom value. A google search will reveal the solution of switching to :edn. But since the custom value runs through the "default" handler of :edn there is no correct way to re-build it when reading the cache. Instead it will just be a placeholder TaggedValue without meaning. If anyone ever tries to access it we now have a case where it will work without caching but not with a cached value.
I think caching should just gracefully fail instead of adding a broken alternative?
António Monteiro November 7, 2016 at 9:17 PM
Attached patch with fix.
We need a flag to explicitly disable the transit encoding - this is to work around code that creates unencodeable forms for one reason or another. EDN had become more lax about encoding random Objects which allowed this stuff to go under the radar in the past.