Fork me on GitHub
#fulcro
<
2018-07-25
>
liesbeth14:07:42

I’ve created a PR to update semantic-ui-wrapper to semantic-ui-react 0.82.0 and change the generators to accommodate that https://github.com/fulcrologic/semantic-ui-wrapper/pull/5

🙂 12
tony.kay16:07:50

@liesbeth Released as 2.0.1

👏 4
thumbsup_all 4
tony.kay15:07:59

Thanks for the work.

bbss21:07:36

I've just upgraded to 2.6.0-RC5-SNAPSHOT from 2.5.8. But now I am getting Uncaught Error: No protocol method IAssociative.-assoc defined for type cljs.core/Atom: [object Object]

bbss21:07:22

Ah I thought I didn't have a stacktrace from my code but I do, let me see if I can figure it out

bbss21:07:50

Seems merge-component! changed:

(prim/merge-component!
                         (:reconciler app)
                         UiGuide {:ui-guide/lesson lesson
                                  :db/id 1
                                  :ui/step 0} :replace [:root/guide])

bbss21:07:26

I updated because a set-state! seemed to not trigger a re-render and I remember seeing an update about doing lifecycle methods properly, and it seems in 2.6.0-SNAPSHOT the set-state! no longer updates within the callback that does the set-state!. Trying later versions until I hit the assoc bug again.

tony.kay21:07:28

@bbss Thanks for the report…we just did a refactor to deprecate certain names, but that should not have broken anything

bbss21:07:16

Okay, still looking into the different versions. I'll keep you posted.

tony.kay21:07:27

on set-state: It is a direct call through to React now, so it should work as documented by React

tony.kay21:07:50

what do you mean “no longer updates within the callback”?

bbss21:07:01

The callback part is not important I guess. But it used to be that in the clickhandler I could set-state! and then get-state and the get-state was updated to reflect the set-state!

tony.kay21:07:20

I just updated the SNAPSHOT with a fix, try that again for the mege component bug

bbss21:07:45

Will do.

tony.kay21:07:03

Ah, so you’re saying that setState isn’t “immediate”. That is true for React 16+

tony.kay21:07:18

it is a queued thing, in prep for async rendering.

bbss21:07:59

okay, and react 16 is since version 2.6?

tony.kay21:07:07

If you set your React version explicitly to an older version, that might return

tony.kay21:07:26

React 16 will be the default for 2.6, yes, but 15 should still work

bbss22:07:04

had to remove fulcro from cache, my app now renders with 2.6.0-RC5-SNAPSHOT, but differently.

bbss22:07:17

I'm using react portals, hope that's not affected.

bbss22:07:28

I guess I was already using react 16 as portals are a react 16 feature.

bbss22:07:50

Hmm, something strange happening. Initial page load renders the app, but then after live-reload I get a

Uncaught Error: Assert failed: (component? component)

bbss22:07:22

In my reload remount If I increase the timeout from 1ms to 1000ms it renders again. (no more assert component? fail) I have a setTimeout on my reload re-mount because when I didn't do that I got a websocket error. However the render still does not render one of my components. Inspecting further.

bbss22:07:11

The reason it renders differently is because the previously erroring merge-component! doesn't seem to work anymore.

tony.kay22:07:33

hm… @currentoor it seems tests of merge-component! were insufficient.

tony.kay22:07:46

you’re right, @bbss, my patch to merg-component wasn’t right

bbss22:07:17

sure thing, thanks for being helpful as ever

tony.kay22:07:49

I put things in SNAPSHOT that have not been well-tested…so this time that proved to be a good idea 🙂

tony.kay22:07:59

I updated it…try again

tony.kay22:07:24

BTW, what websocket error?

tony.kay22:07:42

there was a problem with websockets not waiting for establishment, but that should have been fixed a few versions ago

tony.kay22:07:26

I’m actively using websockets in a project, and that fix works for me

currentoor22:07:12

@tony.kay weird, i tested that line’s change in my app, which certainly relies on fulcro calling merge-component and it didn’t throw

bbss22:07:22

Trying again. The websocket error started happening around the time of that update (I think I know which one you are referring to, I reported the issue with load in started-callback not working on slowish ws connections).

tony.kay22:07:47

there is a warning that it is waiting, but that is what you want

tony.kay22:07:59

@currentoor it is the ! version…you didn’t swap on state

tony.kay22:07:15

so it was just getting an atom instead of state, and doing nothing

currentoor22:07:37

@tony.kay i understand that, i’m just surprised that my app worked despite that bug

tony.kay22:07:52

I don’t see how it could have

tony.kay22:07:00

oh…used the new version?

bbss22:07:04

Oh yes, I get that sometimes when my app errors, I think that fix actually did work. But I am back in Europe nowadays, which is where the "slow" server connection comes from so haven't been able to verify from Korea where I usually live.

tony.kay22:07:59

@currentoor would have only showed with integrate ident portion

tony.kay22:07:18

ok, @bbss, so you’re going again?

bbss22:07:35

yes, in a month or so I'll go back. Fairly certain that it did solve the issue I was having though.

currentoor22:07:46

maybe __integrate-ident-impl__ should also have a :pre check to verify it’s an atom?

tony.kay22:07:04

it should not be an atom

bbss22:07:21

Yes, new version merge-component! works again. Cheers!

currentoor22:07:22

sorry that’s what i meant, verify it’s a map

currentoor22:07:53

the wrapper function has that check i think

tony.kay22:07:54

unfortunately, atoms are record which say true for map?

tony.kay22:07:43

it’s an internal implementation detail, so I’m less worried about that, and more worried that there is no automated test covering merge-component!

currentoor22:07:57

yeah that would be nice 😅

tony.kay22:07:09

those are harder to write, since it uses a reconciler

tony.kay22:07:24

well, with when-mocking they are tractable

tony.kay22:07:32

but more fragile

bbss22:07:11

still having the issue with the setState though. Any reading-up I should do about the immediate/async rendering?

bbss22:07:24

re-render of components' this I set-state doesn't happen.

tony.kay22:07:52

that was kind of word salad

bbss22:07:15

hahaha I was reading it back and thinking... Hmm it makes sense in some way..

bbss22:07:20

Let me rephrase..

tony.kay22:07:40

“Think of setState() as a request rather than an immediate command to update the component. For better perceived performance, React may delay it, and then update several components in a single pass. React does not guarantee that the state changes are applied immediately.”

bbss22:07:34

Okay, that explains why the print of get-state right after is not reflected.

bbss22:07:54

I set-state on the this of a component. But that component doesn't re-render after said set-state.

bbss22:07:12

I think it's the componentShouldUpdate. But my react knowledge is not awesome.

tony.kay22:07:20

my guess is you’re messing up…either you’re not changing the state to something new

tony.kay22:07:30

or you’re mis-measuring re-render

tony.kay22:07:40

shouldComponentUpdate is set by Fulcro

tony.kay22:07:51

and it has been changed recently due to the changes…there could be a bug

tony.kay22:07:18

let me glance at it

tony.kay22:07:12

it looks right to me…it says “yes” if props, state, or children changed

tony.kay22:07:43

and I’ve done set-state in my own demo code and it updated

tony.kay22:07:59

NOTE: There is a callback parameter to set-state that will be called after the state change is applied

bbss22:07:17

I'll check that callback too

bbss23:07:59

The set-state! callback gets called, and the state is updated and changed when I call get-state in that callback.

bbss23:07:22

But no re-render triggered on the component who's this I passed to the set-state! call.

tony.kay23:07:54

I’m compiling the book demos with Rc5

bbss23:07:27

My fear is it is something to do with the react portal.

tony.kay23:07:52

possible I guess

tony.kay23:07:11

so, I’ve got some regressions on the book demos…some of the old libs I was demoing don’t work with newer react

bbss23:07:30

I notice something kind of strange. After I run the callback with the problematic set-state!. I get

[679.214s] [fulcro.client.primitives] integrate-ident is deprecated and will be removed in the future. Please use fulcro.client.mutations/integrate-ident* in your mutations instead.
for about 30-40 times in rapid succession.

bbss23:07:53

200-1500ms difference between the logs

bbss23:07:47

pressing a different button does trigger a re-render that does reflect the state from the problematic set-state!.

tony.kay23:07:47

the warning was just added. We’re moving that function, since it is more of a mutation helper

tony.kay23:07:05

are you calling it that much?

bbss23:07:55

Possibly, not sure where I call it. I'm "replaying" edits to a codemirror instance. I think it's from there.

bbss23:07:29

Ah, no I think it's from elsewhere, one of my push-handlers.

tony.kay23:07:46

I’m a little stumped, and it turns out I’ve got some regressions in the book demos, so I should fix those first…some of them are old versions of demo stuff that requires react 15.

bbss23:07:51

I'm also a little stumped but I'm sure I'll find a way (hacky) to make it work. Tried .forceUpdate on the this. But that doesn't seem to do it. So it's probably not the shouldComponentUpdate then.

tony.kay23:07:27

yeah, forceUpdate bypasses that

bbss23:07:24

I think I got it.. And it ended up to be what you said: "you’re messing up" measured the wrong re-render.

bbss23:07:37

Ugh, I should get to bed. 😄