This issue is fixed in master now thanks to Thomas Heller. The performance hit is negligible.
Thank you for the report. David On Tue, Oct 18, 2016 at 3:45 PM, John Szakmeister <j...@szakmeister.net> wrote: > On Tue, Oct 18, 2016 at 5:59 AM, Thomas Heller <th.hel...@gmail.com> > wrote: > > Don't think there is a best practice for your case in particular. > > Okay. Earlier in the thread you said "while this issue can be very > confusing you will hardly ever run into it when following best > practices." So it seemed to me that perhaps I missing something > important, or didn't know about a best practice that would have saved > me. I think the issue here is a bit different though. There's > nothing that would have saved short of knowing that js->clj has this > issue lurking in the background. :-) > > > The issue is that js->cli is built on top of protocols to allow > converting > > custom JS types to CLJS types. Which makes it extensible for the price of > > checking protocols. In your case you are converting JSON which cannot > have > > custom types, so a custom converter only checking for the very few > possible > > JSON types would "fix" your problem (and would probably be a lot faster). > > Yep. OTOH, I don't really need the performance and js->clj would have > been just as nice to use, if it worked correctly. :-) > > > The case can be made that cljs-ajax should not be using js->clj when > > converting JSON, maybe even add a json->clj to cljs.core. > > I think having json->clj would be a nice option. Though for > cljs-ajax, we probably need to roll our own since the api wouldn't be > available in older ClojureScript versions. > > > The sentinel is the "safest" solution but impacts the performance of > > everyone, so we should be doing more benchmarks on more platforms before > > deciding anything. Benchmarks and Votes on the Jira Issue would help to > push > > this along. > > True, but I think the correctness argument here is pretty valuable. > Again, much of what I see out there is the js->clj is the way to > convert data, and having this problem lurking seems like setting folks > up for failure. I'm pretty persistent when it comes to > troubleshooting this stuff, but I could see others being awfully > frustrated by the result. So I hope that some version of the sentinel > patch makes it in. :-) > > -John > > -- > You received this message because you are subscribed to the Google > Groups "Clojure" group. > To post to this group, send email to clojure@googlegroups.com > Note that posts from new members are moderated - please be patient with > your first post. > To unsubscribe from this group, send email to > clojure+unsubscr...@googlegroups.com > For more options, visit this group at > http://groups.google.com/group/clojure?hl=en > --- > You received this message because you are subscribed to the Google Groups > "Clojure" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to clojure+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "Clojure" group. To post to this group, send email to clojure@googlegroups.com Note that posts from new members are moderated - please be patient with your first post. To unsubscribe from this group, send email to clojure+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/clojure?hl=en --- You received this message because you are subscribed to the Google Groups "Clojure" group. To unsubscribe from this group and stop receiving emails from it, send an email to clojure+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
