This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2018-01-26
Channels
- # arachne (80)
- # beginners (76)
- # boot (16)
- # cider (66)
- # cljs-dev (62)
- # cljsjs (1)
- # clojure (106)
- # clojure-dev (5)
- # clojure-greece (2)
- # clojure-italy (9)
- # clojure-russia (1)
- # clojure-spec (61)
- # clojure-uk (130)
- # clojurescript (21)
- # core-async (9)
- # cursive (3)
- # datomic (37)
- # events (41)
- # figwheel (31)
- # fulcro (27)
- # hoplon (1)
- # jobs (2)
- # lumo (11)
- # off-topic (155)
- # re-frame (71)
- # reagent (27)
- # ring-swagger (3)
- # shadow-cljs (132)
- # spacemacs (5)
- # specter (1)
- # sql (37)
- # test-check (10)
- # uncomplicate (5)
- # unrepl (2)
- # yada (3)
FWIW, I found that completely avoiding shell scripts for "build logic" and instead just writing it in Clojure made the above much easier and cleaner to pull off.
for sure (I’ve started doing some of the same around the clojure api docs)
@dnolen If you end up doing "ClojureScript Friday" work, I'd vote to get the map entry stuff in, but only if there is not an immediate release also planned. (The map entry stuff seems solid, but every now and then we still find a new corner case to address.)
@dnolen Here is the list of the follow-on map entry tickets I was looking at / testing with: https://dev.clojure.org/jira/browse/CLJS-2460 https://dev.clojure.org/jira/browse/CLJS-2477 https://dev.clojure.org/jira/browse/CLJS-2478 https://dev.clojure.org/jira/browse/CLJS-2482 with the commit for https://dev.clojure.org/jira/browse/CLJS-2456 and the 4 above, everything was working for me
Sorry, @dnolen I’m trying that simple change… but it is failing. Attempting to figure out why.
TypeError: this.mseq.cljs$core$ISeq$_first$arity$1(...).cljs$core$IMapEntry$_key$arity$1 is not a function
at cljs.core.KeySeq.cljs$core$ISeq$_first$arity$1 (builds/out-simp/core-simple-test.js:1676:411)
at Object.cljs.core.first (builds/out-simp/core-simple-test.js:534:165)
at Object.cljs.core.zipmap (builds/out-simp/core-simple-test.js:1756:213)
at Object.cljs.pprint.map_params (builds/out-simp/core-simple-test.js:2840:144)
at Object.cljs.pprint.compile_directive (builds/out-simp/core-simple-test.js:2842:284)
at Function.<anonymous> (builds/out-simp/core-simple-test.js:2870:205)
at Function.cljs.core.apply_to_simple.cljs$core$IFn$_invoke$arity$3 (builds/out-simp/core-simple-test.js:897:157)
at Function.cljs.core.apply_to_simple.cljs$core$IFn$_invoke$arity$2 (builds/out-simp/core-simple-test.js:896:188)
at Function.cljs.core.apply.cljs$core$IFn$_invoke$arity$2 (builds/out-simp/core-simple-test.js:909:244)
at Object.cljs.pprint.consume (builds/out-simp/core-simple-test.js:2209:107)
This causes:
Error: No protocol method IMapEntry.-key defined for type cljs.core/PersistentVector: [:a 1]
at Object.cljs$core$missing_protocol [as missing_protocol] (/Users/mfikes/Projects/clojurescript/out/.cljs_node_repl/cljs/core.js:326:9)
at Object.cljs$core$_key [as _key] (/Users/mfikes/Projects/clojurescript/out/.cljs_node_repl/cljs/core.js:2000:17)
at Object.cljs$core$key [as key] (/Users/mfikes/Projects/clojurescript/out/.cljs_node_repl/cljs/core.js:29149:18)
at /Users/mfikes/Projects/clojurescript/out/.cljs_node_repl/cljs/core.js:33326:32
at Object.cljs$core$pr_sequential_writer [as pr_sequential_writer] (/Users/mfikes/Projects/clojurescript/out/.cljs_node_repl/cljs/core.js:32530:142)
at Object.cljs$core$print_prefix_map [as print_prefix_map] (/Users/mfikes/Projects/clojurescript/out/.cljs_node_repl/cljs/core.js:33325:18)
at Object.cljs$core$print_map [as print_map] (/Users/mfikes/Projects/clojurescript/out/.cljs_node_repl/cljs/core.js:33346:18)
at cljs.core.PersistentArrayMap.cljs$core$IPrintWithWriter$_pr_writer$arity$3 (/Users/mfikes/Projects/clojurescript/out/.cljs_node_repl/cljs/core.js:33573:18)
at Object.cljs$core$pr_writer_impl [as pr_writer_impl] (/Users/mfikes/Projects/clojurescript/out/.cljs_node_repl/cljs/core.js:32709:12)
at Object.cljs$core$pr_writer [as pr_writer] (/Users/mfikes/Projects/clojurescript/out/.cljs_node_repl/cljs/core.js:32830:18)
Yes… that is part of the change in CLJS-2478. So, I took patch 2 from that ticket and replaced satisfies?
with implements?
in the map-entry?
predicate.
OK… yeah. I would have thought implements?
would have had only a perf effect, not derailing functinoality.
searching for current places where we call (satisfies? IMapEntry ...)
should be helpful
If we allow vectors to remain as map entries (and they satisfy the map-entry?
predicate), there are two places that, in hindsight, I think I was assuming they wouldn’t be, and now I’m re-questioning correctness:
https://github.com/clojure/clojurescript/blob/d1dc3c53c778772b176fc2163bf090f56e2a75e6/src/main/cljs/clojure/walk.cljs#L47
https://github.com/clojure/clojurescript/blob/d1dc3c53c778772b176fc2163bf090f56e2a75e6/src/main/cljs/cljs/core.cljs#L10542-L10543
so stick with vectors are not IMapEntry
, and fix those places where Clojure takes them to also take them (not just check IMapEntry)
to prevent breakage we can actually extend vector to IMapEntry
(not inline, in order to ensure implements? IMapEntry
fails for vectors)
but internally we going to separate the checks into implements? IMapEntry
+ vector?
check
Cool. I want to understand the aspect about TransientArrayMap
needing to support vectors and how just using implements?
causes things to fail. I'm assuming this reflects the logic you have in mind above.
Cool. I'll get there in a second. 🙂 I think your understanding is a step or two ahead of mine right now . 😜
3. Any previous place where vectors were treated as IMapEntry
must now handle vectors explicitly
4. To prevent breakage, we should extend-type
vectors to IMapEntry
(`satisfies? will work
implements?` won’t)
Cool. There is evidently still a place that falls under (3) that is not being addressed in the patch. (The fallout of how zipmap
behaves.)
OK @dnolen I sorted out what was going on. It was a CM error on my part. CLJS-2478-2.patch is designed to be applied after CLJS-2456.patch. I mistakenly went into the local repo where I had produced CLJS-2478-2.patch, changed satisfies?
to implements?
and simply ran the tests. The tests failed because that repo did not have CLJS-2456.patch applied. The failure essentially took the form of where a seq
on a map would return vectors, not map entries, and the print code (correctly) expects map entries and was applying key
. Sorry for the long explanation, but this CM screw up partly explains why I was at a complete loss as to understand what was going on. TLDR: Changing satisfies?
to implements?
results in the patch working, as you would expect. Will attach it to the ticket now.
cljs.user=> 100/3
clojure.lang.ExceptionInfo: failed compiling constant: 100/3; class clojure.lang.Ratio is not a valid ClojureScript constant. {:constant 100/3, :type clojure.
lang.Ratio}
should we support emitting these?