@wilkerlucio trying out the latest fulcro-inspect. It seems to work for me, except i’m getting odd multimethod errors about :com.wsscode.pathom.connect/indexes. Fulcro-inspect appears to be attempting to use my custom remotes with some query keyed by that keyword above.

i believe it’s hitting all my remotes

@levitanong yes, on initialisation it tries to pull the pathom indexes, but I'm considering remove this auto-load, you are the second person having issues with that

I had the assumption that peoples parsers would just ignore unknown keys, but seems like it's not the case

heh…that was a rather big assumption 😄

yeah, I'll try to make a new release today fixing that, another big one is that I want try an hybrid encoding mode

instead of trying to "fix" the transit compilation by walking, I wanna see if I can just encode via EDN when transit fails

and see how that goes, so both can be a release soon (maybe later today)

@levitanong is that blocking you from using it? or just an annoying error?

@tony.kay we’re nearly onto fulcro, I saw you’ve been answering nicks questions thanks a lot for the help, i got pulled in for a couple remainder issues,

#(transact! T [(listing-details/upload-images {:parentId _id

(defmethod fulcro.client.mutations/mutate :default [env k params]

it’s worth mentioning that it’s not that the key no longer exists in env, it’s present, but just NIL

is this a bug?

Hey @jasonjckn. I just found a bug yesterday where lazy seqs are not getting forced

is this in release only, or in dev, too?

only tested in dev so far

ok, so that’s not it 😕

not ringing a bell…pretty sure component is still added to env internally…let me dbl check

it's strange, I use those in Fulcro Inspect, and I'm seeing the component information present on results

should be pretty easy to reproduce , :component is nil for all mutations I see getting hit

yep. being addded

@jasonjckn are you using fulcro inspect?

no

if you do, you can try checking if the component shows up in the transaction history

just did a js/console.log

(defmethod fulcro.client.mutations/mutate :default [env k params]
  (js/console.log "fulcro.mutate")
  (js/console.log env)

https://github.com/fulcrologic/fulcro/blob/develop/src/main/fulcro/client/primitives.cljc#L2409

can’t see how component would be nil and transact still work

is not :action missing there?

parser called on 2416

oh, in fact it is 😄

but even then env should be ok

hmh yah code looks good.. weird not sure why i’m seeing this

pasted a screenshot can you guys see it ?

Your file was uploaded — it's safe and sound in Slack. Unfortunately your workspace doesn't have any storage space left. To get more space, you can upgrade to a paid account or delete some of your older files

you sure you’re passing a component?

to transact!

yes

could you log that out at the transact call site?

ok,

OH…is your component missing a query?

https://github.com/fulcrologic/fulcro/blob/develop/src/main/fulcro/client/primitives.cljc#L2487

if it is a stateless component, then :component isn’t set

because technically then that component isn’t in any indexes, and isn’t safe to use for certain ops I think.

yes no query

this must have changed in fulcro then

basically transact! walks up the tree looking for a stateful component to run the tx against…:component will be set to the first component above it that has a query

that logic came from Om Next

but it is possible something got changed

seems like a reasonable request to know which component was transacted against?

actually, the walking to the parent code might be unreachable

and it’s not returning a parent component, it has nil for the component

right…what I just said…I think has-query? might have been recursive in the past…e.g. me nor any parent has a query

ah

so, “which” component was “transacted against” is a good question in terms of purpose…the transactions are related to state, which only ends up in the UI via queries.

which is the reasoning behind :component being something that has a query

the parent of this component has a query

seems like you could reason :component being the parent component, or the one it was transacted on

but currently it’s nil , which makes no sense to me

it is nil because the logic looks broken on that scan

the cond tests if it has a query, and then just runs it with a nil component

so, I can see why it is nil…I was talking about the intended (apparent) behavior

I’d have to look at Om Next’s source to verify my assertions there

about intended behavior

https://github.com/omcljs/om/blob/master/src/main/om/next.cljc#L1594

nope…looks like the same problem there

same logic…so, IF there is no query on the component, it looks to me like Om Next/Untangled/Fulcro will all make :component nil

hmm, i’m quite positive this has been non-nil in the past

so something changed somewhere.. but yah not clear where

When I ported that I was looking for max compatibility. I could be convinced that the logic is wrong, since it looks weird to me

If it HAS a query, why would you need to recursively look for a parent with a query

I think the logic is just wrong…Also, I tend to agree with you: it seems harmless to pass the component into mutation, even if it has no query

yah we certainly put that information to good use

oh…re-rendering logic: The transact wants to refresh the component whose data changed…which MUST be a component (in Fulcro) with a query and ident

otherwise it is forced to do a root render

and that component gets “queued” if present

but you would not want to queue a stateless component, since there is no way to refresh it in isolation

yes, I believe we can make do with :component = parent component

i definitely understand the reasoning for that

but in that case you’re supposed to use callbacks

to run the transact via the stateful parent

but it looks like the logic in place is meant to scan for the parent, so I think either should work

it just doesn’t because the logic is wrong 😜

let me try a fix…just a min

@jasonjckn Try 2.5.8-SNAPSHOT

it isn’t the parent component that will get passed in…it will be the correct one. The parent logic is for tx intercepters, which are not even documented

Nice, love it

one can always crawl the tree downward if you need to

so you have maximal information this way

upward? You will get the exact component that transacted, no matter what

the parent traversal wan’t changing :component, it is propagating messages to components that are parents that implement that protocol

which no one does, because it is an undocumented/supported feature 🙂

sadly i’m running into a bunch of issues trying to move from 2.4.4 to 2.5.x , not sure I can test right now on 2.5.8-SNAPSHOT, but this patch gets my thumbs up

I think there’s more stringent checks on defui in 2.5.x (shouldComponentUpdate [T next-props next-state next-context] is failing to compile

it’s using React’s Context feature

using defui?

yes

validate signatures has not changed

defsc might think it incorrect

well, both should actually

since both try to validate signatures

can I disable the extra spec validation?

shouldComponentUpdate [T next-props next-state next-context] is valid in react

hm…let me look into it for a sec

what version supports next-context?

this was an experimental feature in react v15.x trying to check the status of this in react v16.x

I see no docs saying that is a legal method sig

https://reactjs.org/docs/legacy-context.html

so yeah…not sure I want to do that

why do you need it?

so the old API is still supported

but deprecated

React previously shipped with an experimental context API. The old API will be supported in all 16.x releases, but applications using it should migrate to the new version

we built our own forms ontop of this feature

oof

what is it used for, specifically?

well the feature itself sort of works like clojure’s bindings and dynamic vars except instead of call stacks it’s child-parent components, and props instead of vars

so you can pass props from parent to child implicitly with a context

so specifically in our forms, we a bootstrap form-group that sets up a context

and all child nodes, e.g. bootstrap submit button, text fields, etc can see this context

does this context change after bootstrap?

you can change it yah , not sure I follow

trying to figure out if shared is appropriate alternative

well there’s a new conetxt api in react v16 with different signature than the legacy api

it’s a feature that gets used enough

https://reactjs.org/docs/context.html

but it doesn’t affect the lifecycle sigs I don’t think

right not anymore

so I get why you don’t want to support this, it’s non trivial to change this part of our app right now

so we’re still on 2.4.4

so port to that and it’ll work…but not sure how you ever got it past Om Next

or 2.4.4 of Fulcro

fulcro

that validation has been there for years I think

hmh, thought this error only showed up in 2.5.x

i will need to confirm though

yes

https://github.com/omcljs/om/blob/master/src/main/om/next.cljc#L71

Caused by: java.lang.AssertionError: Assert failed: Invalid signature for shouldComponentUpdate got [T next-props next-state next-context], need [this next-props next-state]
(= (count sig') (count sig))
	at fulcro.client.primitives$validate_sig.invokeStatic(primitives.cljc:218)
	at fulcro.client.primitives$validate_sig.invoke(primitives.cljc:216)
	at fulcro.client.primitives$reshape$reshape_STAR___15608.invoke(primitives.cljc:424)
	at clojure.core$map$fn__5587.invoke(core.clj:2745)
	at clojure.lang.LazySeq.sval(LazySeq.java:40)
	at clojure.lang.LazySeq.seq(LazySeq.java:49)
	at clojure.lang.RT.seq(RT.java:528)

So all versions of Untangled and Fulcro should have balked

hmh i see

weird

ok, i’ll keep digging , but only seeing this compiler error in 2.5.x so far

you could elide asserts

ah good idea/hack

I don’t know if “good idea” is how I’d describe that 😜

haha yah

hence the edit

“you could ignore all exceptions” good idea! XD

in any case, your patch for 2.5.8 gets my vote

prob won’t be able to test it for a while though

in the interim I may just add queries everywhere that’s affected by this, but will be happy to upgrade when we can

OH…I know why it is validating now…I made it possible to override shouldComponentUpdate

rather…I added a wrapper

you could override it before, but the latest versions give you next-props and next-state of Fulcro…not low-level React

and adding that wrapper caused it to start validating

@jasonjckn an easiler workaround is to call transact* directly

(transact* (get-reconciler this) this nil tx)

make yourself a little wrapper function

@tony.kay > “you could override it before, but the latest versions give you next-props and next-state of Fulcro…not low-level React” ok, any other ideas besides eliding asserts for this one