Fork me on GitHub
#fulcro
<
2018-03-12
>
tony.kay08:03:56

@bbktsk it is possible you found something I planned, but didn’t implement 😜

tony.kay08:03:21

@magra refreshes are based on queries, not what is in the db. Also, for a refresh to work, there has to be something to refresh…if the thing is just coming onto the screen then it won’t be in an index yet. You’ll need to trigger a refresh on the query above it.

levitanong08:03:39

@tony.kay fulcro 2.3.2, fulcro-spec 2.0.3-1

tony.kay08:03:28

Refresh rules are very simple: 1. The component that ran the tx refreshes. 2. Any on-screen component that queries for a kw you’ve indicated a refresh on will refresh (but this is refresh, so you’d want to hit the parent for a new item).

tony.kay08:03:57

@levitanong version of clj/cljs?

tony.kay08:03:21

strange. The assertions block and throws arrow just translate to underlying clojure test stuff. The symbol it is complaining about is one defined by those lower-level libs, which should be included when you pull in fulcro-spec.core

levitanong08:03:49

Hmm. Also, I got Wrong number of args (1) passed to cljs.core/ExceptionInfo

tony.kay08:03:05

ah, that is probably the more likely culprit

tony.kay08:03:16

try using a map on the RHS of throws arrow.

tony.kay08:03:42

with a :regex key, I think, and a regex (that’ll let you match against the exceptions message)

tony.kay08:03:52

it’s a weird notation on that one

levitanong08:03:31

That got rid of the wrong args warning, but the undeclared var of throws? still happens

levitanong08:03:46

Use of undeclared Var my-project.util-spec/throws?

tony.kay08:03:18

I don’t have other ideas.

tony.kay08:03:01

try putting a require for clojure.test in your file

levitanong08:03:56

the error message tells me that the assertions macro generates throws? but in my own namespace instead of the clojure.test ns

levitanong08:03:03

at least i think that’s what it means

tony.kay08:03:30

it’s used all over the place in the tests in fulcro itself.

tony.kay08:03:56

although those are all cljc and clj files…

tony.kay08:03:35

perhaps there is a problem if you’re just using cljs, and it doesn’t pull in the macros (which are clj)

levitanong08:03:58

ah, i had thought about this, so i changed the require statement to go :include-macros true

levitanong08:03:01

but still no dice

levitanong08:03:39

also i’m on cljc. haha

tony.kay08:03:15

this is the code in spec itself (a cljs file):

(ns fulcro-spec.assertions-spec
  (:require [fulcro-spec.core :refer [specification assertions]]))

(specification "assertions blocks work on cljs"
  (assertions
    "throws arrow can catch"
    (assert false "foobar") =throws=> (js/Error #"ooba")
    "throws arrow can catch js/Objects"
    (throw #js {}) =throws=> (js/Object)))

tony.kay08:03:51

no clojure.test required

levitanong08:03:03

how perplexing.

levitanong08:03:35

i’m looking through the clojure.test api, i don’t see a throws?

levitanong08:03:01

not in the clojure.core namespace either.

tony.kay08:03:55

Oh wait...shadow-cljs?

levitanong08:03:47

the version i’m using is 2.2.6

tony.kay08:03:11

ok, there was a problem with shadow-cljs giving an invalid warning on spec, now that I think about it

tony.kay08:03:24

so, you're not going crazy 🙂

tony.kay08:03:57

I had to rewrite something in spec to fix that...I don't remember what, but if you're using the latest version I would have expected it to work 😕

levitanong08:03:53

well, it could still be an unrelated problem

tony.kay09:03:23

no, see there is some code to add addl support to the clojure.test is expression internally

tony.kay09:03:32

and it adds throws?

tony.kay09:03:12

and what it was doing was a case against some symbols, if I remember right...and that was screwing up so I changed it to a cond, and I thought that fixed it

levitanong09:03:30

perhaps the problem is in the symbol being interpreted as a var rather than as a symbol

tony.kay09:03:10

well, it is obviously different between it and standard cljs, cause it works on the standard compiler 🙂

thheller09:03:44

what is up?

thheller09:03:50

I hear bugs?

tony.kay09:03:06

Hey Thomas 🙂

tony.kay09:03:05

I had a difference between the main cljs compiler and shadow. Fixed in that line 88 reference above. Using symbols in a case statement wasn't working for me in shadow, so I changed it to a cond. That seemed to fix it for me, but @levitanong is reporting he's having similar problems still.

levitanong09:03:20

i’ll try to repro off a new template

thheller09:03:29

the commit you linked has a lot of whitespace changes. hard to figure out what you actually changed

tony.kay09:03:38

that's why I said line 88 🙂

tony.kay09:03:44

case vs cond

tony.kay09:03:08

when using the case, shadow was resolving the symbol in the user's ns, instead of as a raw symbol

thheller09:03:34

hmm it shouldn't change how case works but I'll look into it

levitanong09:03:04

yup it’s happening. on a fresh fulcro shadow-cljs template, adding this triple: (throw #js {}) =throws=> (js/Object) causes

8 |     (assertions
-----------^--------------------------------------------------------------------
 Use of undeclared Var test-repro.sample-spec/throws?
--------------------------------------------------------------------------------
   9 |       "with positive integers"
  10 |       (+ 1 5 3) => 9
  11 |       "with negative integers"
  12 |       (+ -1 -3 -5) => -9
--------------------------------------------------------------------------------

thheller09:03:57

isn the cljs.test assert thrown? not throws?

tony.kay09:03:06

and when you look at your resolved deps, you're actually getting the latest spec, right?

tony.kay09:03:34

fulcro-spec defines it's own I think. I didn't write a lot of that code, so I'm not for certain 😜

tony.kay09:03:26

yeah, line 140

tony.kay09:03:39

you can extend the support of is using a multimethod

tony.kay09:03:04

that code has been working fine in regular cljs since before the 1.9 series

levitanong09:03:47

@tony.kay yeah, on lein deps :tree, it’s showing i have 2.0.3-1

tony.kay09:03:57

yeah, I can swear it was fixed for me when I changed the case statement. Strange.

thheller09:03:30

can't find anything wrong with the case statement?

tony.kay09:03:27

it's 2am here and I'm kinda brain dead. I don't understand your question Thomas

tony.kay09:03:25

The reproduction of the problem, which only happens on shadow, is: 1. lein new fulcro app 2. shadow-cljs watch test 3. Edit the sample spec, and add to the assertions block:

(assertions 
   "some test of throw"
   (throw #js {}) =throws=> (js/Object))

tony.kay09:03:17

that code I pointed you to is the implementation of the assertions macro

tony.kay09:03:29

I'm going to head to bed. Thanks for checking in @thheller 🙂 If you see something that is technically wrong in fulcro-spec and just accidentally works in the standard compiler, Iet me know.

thheller09:03:08

@tony.kay I'll check out and and fix it

levitanong09:03:16

thanks also guys. have a good night, @tony.kay!

thheller09:03:30

(ns app.sample-spec
  (:require
    [fulcro-spec.core :refer [specification provided behavior assertions]]))

; Tests for both client and server
(specification "Sample Spec"
  (behavior "addition computes addition correctly"
    (assertions
      "some test of throw"
      (throw #js {}) =throws=> (js/Object)
      "with positive integers"
      (+ 1 5 3) => 9
      "with negative integers"
      (+ -1 -3 -5) => -9
      "with a mix of signed integers"
      (+ +5 -3) => 2)))

thheller09:03:54

thats whats supposed to fail right?

thheller09:03:30

compiles fine with 2.0.1 even?

levitanong09:03:10

i don’t understand the question haha

thheller09:03:34

it is compiling/running fine for me with 2.0.1 and 2.0.3-1

levitanong09:03:11

i have a hunch

thheller09:03:19

ahh I think I know

thheller09:03:32

it only fails for cached builds

levitanong09:03:09

cached builds?

levitanong09:03:21

you mean to say lein clean should fix it?

thheller09:03:44

lein clein shouldn touch the cache anymore

thheller09:03:09

but yes you can try to delete rm -rf .shadow-cljs/builds/test

thheller09:03:27

hmm or not .. now I can't reproduce anymore

levitanong09:03:28

well, deleting the cache worked for me. @[email protected]

levitanong09:03:43

it’s weird, the problem was happening on a repro project that i had started up but didn’t do anything with

levitanong09:03:22

or rather, the problem was happening on my work project, i reproduced the problem on a repro project i spawned a couple of days ago but didn’t do anything with

levitanong09:03:39

but couldn’t reproduce it on a fresh template

thheller09:03:05

yeah I just saw it after lein new fulcro so there definitely is an issue somewhere

levitanong09:03:50

Yay I'm not crazy

thheller09:03:41

it was fixed but is not in any official release yet

thheller09:03:53

I'll try to bump to the latest 1.10 pre-release

thheller09:03:39

what bothers me that it only happened once and now I can't reproduce it

thheller09:03:04

caching is already pretty defensive and pretty much always invalidates whenever anything changes

thheller11:03:50

so weird .. I cannot get the error to appear again

levitanong12:03:50

lol it came back

levitanong12:03:34

but again, i can’t repro. What happened when it happen was: 1. I restarted my computer 2. I ran lein repl, showing some error message because i had forgotten a require. After fixing it, i ran shadow-cljs watch test, then the problem happened. 3. had to clear the cache before it started working again. I restarted again, tried shadow-cljs watch test and the problem didn’t show up anymore.

mitchelkuijpers16:03:58

Anyone know why shadow-cljs and fulcro-spec give me these errors which will only go away after blowing away all shadow-cljs state:

------ WARNING #1 --------------------------------------------------------------                    
 File: /home/mitchel/Development/atlas-crm-next/test/atlas_crm/ui/components/entity_selector_test.cljc:38:8
--------------------------------------------------------------------------------            
  35 |      (let [result (run-mutation {:state temp-idents-state                
  36 |                                  :mutation-key `entity-selector/close-autocomplete
  37 |                                  :ref ident})]                                                      
  38 |        (assertions                                                                                   
--------------^-----------------------------------------------------------------
 Use of undeclared Var atlas-crm.ui.components.entity-selector-test/exec
--------------------------------------------------------------------------------                                                               
  39 |          "Temp data is gone"                                                                         
  40 |          (get-in result [:item/by-id 99]) => nil                 
  41 |          "Other results still exist"                                     
  42 |          result =fn=> (*contains? {:item/by-id {1 {}             
--------------------------------------------------------------------------------

thheller16:03:15

@mitchelkuijpers trying to figure that out. proving really hard to reliable reproduce

myguidingstar17:03:14

hi all, walkable[1] now support many dbms by specifying quote marks eg "\"" for postgres, "`" for mysql

myguidingstar17:03:53

latest version (1.0.0-SNAPSHOT) is on clojars

myguidingstar17:03:34

your feedback is most welcome

magra18:03:16

@tony.kay Thank you. I tried again the whole day. I must be missing something basic. Not even :render-mode :brutal works. (:render-mode (:config (:reconciler @inserate.client/app))) returns :brutal so I should have hit the right switch, right? All adding to existing lists and children and editing works. But adding a first child-item just adds to the db, not to the screen. Did I block an internal mechanism or am I just barking up the wrong tree? Any ideas, what I might be missing?

magra18:03:28

I followed your example of the top-router and report-router. What would be the syntax to force the report-router to pick up changes in the db?

thheller18:03:17

@tony.kay so I identified the fulcro-spec issue. the issue is that the macro does a side-effect on the CLJ side. when the CLJS is cached only the CLJ side of the macro is executed but the macro itself is not called. I would recommend rewriting the macro without all the if-cljs and just always do the clojure.test/assert-expr AND cljs.test/assert-expr defmethods

thheller18:03:44

it works in CLJS since it doesn't cache ...

thheller18:03:03

other option is that I disallow caching for fulcro-spec entirely

thheller18:03:35

you could probably just do the defmethod inside a :clj read-cond entirely without a macro

tony.kay18:03:35

@thheller ooof. OK, I’ve been seriously thinking about rewriting the whole thing. I wrote the original outline, but it was put together from there by others, so I’m not too familiar with the code base. Thanks for looking at it. Sorry it sucked up so much of your time!

thheller18:03:53

the entire thing does a whole lot of macro magic although it really doesn't have to

thheller18:03:25

but yeah side-effecting macros don't work when caching is involved

mitchelkuijpers18:03:51

I think this will also break without shadow-cljs because they also want to build more caching inside clojurescript

thheller18:03:23

it should already break if you have :cache-analysis true enabled but yes even more of a problem for the newer shared AOT cache

thheller19:03:00

if that is ever going to work in the first place. I tried that for shadow-cljs but quickly backed out of it. cache between builds is just a nightmare to invalidate

tony.kay19:03:18

@magra The refresh story has been around for years, so if you can put up an repro case I can look at it. If you’re doing keyframe and it isn’t working, then your state isn’t right to begin with.

tony.kay19:03:54

chances are your navigate away and back is doing something subtle to the app state that is fixing it

tony.kay19:03:27

@mitchelkuijpers @thheller so, all the more reason to clean it up now

tony.kay21:03:57

@thheller The problem is the multimethod in cljs has a different signature than the one in clj 😕

tony.kay21:03:35

so you have to define the multimethod differently dependning on which context you’re in…I think I might have to rewrite it not to use that multimethod at all

thheller22:03:10

just (do (defmethod clojure.test/assert-expr '= [msg form] ...) (defmethod cljs.test/assert-expr '= [menv msg form] ...))

thheller22:03:27

absolutely no need for a macro or any magic

thheller22:03:54

in a #?(:clj (do ...)) so it doesn't happen in cljs of course

thheller22:03:25

the multimethod is called during macro expansion so its always pure clojure

tony.kay22:03:41

right, I know that, but…mind twisting.

thheller22:03:29

yeah its a bit weird when thinking about clj/cljs macroexpansion at the same time

wilkerlucio22:03:37

I think making a separated multi-method makes it easier to understand, mixing arities depending on clj/cljs seems like a mind bugger

tony.kay22:03:08

I very much agree that this is twisted difficult to understand code

thheller22:03:55

to me pretty much everything in .cljc is twisted difficult to understand code 😛

thheller22:03:04

my mind is not made for thinking in 2 platforms at once

wilkerlucio22:03:31

agreed, and that's even more reason to avoid doing extra complications that can be avoided

tony.kay22:03:00

@levitanong try 2.0.4-SNAPSHOT of fulcro-spec. I think that fixes it

cjmurphy23:03:48

@magra :keyframe would be the one you want to try not :brutal. Also with your example before you had (:company/_offices params) to get the id part of an Ident, which looked odd to me.

tony.kay23:03:32

New release of lein template. Updates fulcro-spec to work better against shadow-cljs

tony.kay23:03:41

Released 2.0.4 of fulcro-spec