Things like `java-time[.api]/truncate-to` do not work correctly when underlying protocol is extended
camsaul opened this issue · 5 comments
In Metabase we extended the Truncatable protocol with some tweaks that we needed to handle certain situations:
Previously, we were using a fork of 0.3.3, where the java-time namespace used Potemkin import-vars, which meant that java-time/truncate-to would be updated when the underlying java-time.core/truncate-to var changed (when using extend/extend-protocol/etc.
The newer version of this namespace just uses def to capture the value of java-time.core/truncate-to when the namespace is loaded; any subsequent changes to the underlying var no longer cause java-time/truncate-to or java-time.api/truncate-to to be updated.
I spent a little while trying to debug
java.lang.IllegalArgumentException: No implementation of method: :truncate-to of protocol: #'java-time.core/Truncatable found for class: java.time.LocalDate
errors in Metabase after upgrading to the latest version of this library until I figured out what was going on.
Switching back to Potemkin would prevent others from running in to issues like these. Alternatively, instead of something like
(ns java-time.api ...)
(def truncate-to java-time.core/truncate-to)maybe you could do
(def truncate-to #'java-time.core/truncate-to)or even
(defn truncate-to [t unit] (java-time.core/truncate-to t unit))so the var gets resolved each time you use it.
Thanks for the detailed report. I'll think it over.
Notes:
- the root cause is https://clojure.atlassian.net/browse/CLJ-1796
- I wonder how this interacts with direct linking.
- do extends need to happen before usages in general?
- consider adding
:redefto all public protocol methods
Fixed in 1.2.0.
@frenchy64, no we don't use direct linking