This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2019-01-07
Channels
- # announcements (3)
- # beginners (124)
- # calva (60)
- # cider (81)
- # cljs-dev (65)
- # cljsrn (1)
- # clojure (268)
- # clojure-dusseldorf (2)
- # clojure-europe (3)
- # clojure-italy (9)
- # clojure-losangeles (1)
- # clojure-nl (22)
- # clojure-russia (3)
- # clojure-spec (24)
- # clojure-uk (34)
- # clojurescript (72)
- # code-reviews (8)
- # cursive (20)
- # datomic (32)
- # fulcro (49)
- # jobs (1)
- # jobs-discuss (15)
- # juxt (10)
- # lein-figwheel (10)
- # nrepl (4)
- # off-topic (37)
- # overtone (1)
- # portkey (2)
- # protorepl (8)
- # random (1)
- # re-frame (1)
- # reagent (43)
- # reitit (8)
- # ring (16)
- # ring-swagger (2)
- # rum (6)
- # shadow-cljs (63)
- # specter (2)
- # testing (32)
- # tools-deps (32)
- # unrepl (1)
- # vim (3)
what does the implementation matter as long as you implement the protocol faithfully?
there's a bit of a diamond problem in certain paths if you implement both ISeq and IPersistentVector/Indexed in clojure
don't know if the same problem exists in cljs too but I wouldn't be surprised if it did
now that you’re awake, do you think it would be likely to receive an argument to apply that does not implement INext
?
but usually the pattern is that you make things seqable and then the seq object is just a seq and nothing else
Because in CLJ ISeq has first/next/more as methods, where as ISeq in CLJS has first/rest as methods.
I can't remember why it was different but I think the upshot was: in CLJS if implementing ISeq one should also implement INext to handle the next method. I may be totally misrembering though...
@tmulvaney I’m asking specifically for this patch: https://dev.clojure.org/jira/browse/CLJS-3023
So far I haven’t encountered any problems by rewriting (next ...)
to (-next ...)
in apply-to-simple
. It also passes the Canary tests. I think you could artifically create an ISeq instance that does not implement INext, but I wonder if this will ever happen in practice.
Yeah i think all the core datastructures have INext where it's needed. It would be external data types from libraries etc which may not implement INext.
yes, that’s the question I was wondering about. Alternatively I wrote a version of next that skips the nil?
call, since that has already been established when calling next
in apply-to-simple
.
it fixes the issue, but not sure it’s worth it. also, the performance gain is really nothing. maybe I should just close the ticket.
I assume you meant: libraries that targeted older versions of CLJS and have their own data structures that implement ISeq
but not INext
could potentially give problems in combination with apply
yeah, I meant, when this protocol didn’t exist, users didn’t even have the choice, so it’s likely those exist out there
this could be used in apply-to-simple
instead of next
:
(defn- ^seq next*
(if (implements? INext coll)
(-next ^not-native coll)
(seq (rest coll))))
it skips the nil?
check and also fixes the issue.I think your change to the keyword clojure.test.check
will cause unnecessary headaches in the next release
so accept both as input, but if people use the the new one, the check results should also use the new one, else the old one?
if people do something like: (every? #(:pass? (get % :clojure.test.check/ret)) stc-result)
I haven't yet looked at the stackoverflow in https://dev.clojure.org/jira/browse/CLJS-3030 but I've unassigned it from me (anyone should feel free to take a whack at that one)
@mfikes maybe the text about the keyword change for passing clojure.test.check options should mentioned that the old keyword is still supported as backward compatibility
or not, people won’t notice that it has changed if they don’t read the release notes 🙂
Yeah, I’ll update. We could almost remove the note, but I’ll try revising the language to see how that works.
@mfikes there is one interesting update though. You now have to to require clojure.test.check
and clojure.test.properties
yourself, because this is not done by clojure.spec.test.alpha
anymore (it’s made lazy)
$ clj -m cljs.main -re node
cljs.user=> (require '[clojure.spec.test.alpha :as stest])
cljs.user=> (require '[clojure.spec.alpha :as s])
cljs.user=> (s/fdef clojure.core/inc :args (s/cat :n number?))
cljs.core/inc
cljs.user=> (stest/instrument)
[cljs.core/inc]
cljs.user=> (stest/check `inc)
Execution error (Error) at (<cljs repl>:1).
Require clojure.test.check and clojure.test.check.properties before calling check.