Fantastic! Thank you! -John
On Tue, Oct 18, 2016 at 11:49 PM, David Nolen <dnolen.li...@gmail.com> wrote: > 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. -- 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.
