useFocusEffect / useIsFocused

[slorber]

@satya164 does it feel ok?

Didn't use the use-subscription package for now, didn't know about it but will take a look.

Also didn't invest in tests because there's not yet any test infra, not much time, and this project is probably only temporary until people use the v5. But those hooks will be used in my app right now so I'm pretty confident to see if something breaks.

Will add some doc too.

[slorber]

Hi @satya164 I've made some changes and added some doc

[slorber]

Thanks :) seems ready to merge now

[ArrayZoneYour]

@slorber If I run setParams in callback, navigation will change and invoke callback again, it will raise a infinite loop

[slorber]

thanks for reporting this.

@satya164 how do you handle those loops in v5? I guess we should fix it the same way in v5 and here

[satya164]

v5 doesn't update navigation prop on params update since route is a separate prop. regarding how to handle it in v4, user needs to add a check manually that params are out of date before trying to update them. they should do it anyways to prevent unnecessary updates to the screen regardless of the loop anyways since we don't do any equality checks like setState does.

[slorber]

@satya164 I understand your point about preventing useless re-renders/infinite loops.

But if param value has to change, we still don't necessarily want to re-execute useFocusEffect again (at least user should be in control imho)

I mean:

This one should re-execute on userId param change

useFocusEffect(useCallback(() => { ... },[params.userId]));

But this one should rather not re-execute even if any param changes:

useFocusEffect(useCallback(() => { ... },[]));

So I think it's not correct to make this hook depend on navigation here, as the object is unstable it will tend to produce unwanted effect re-execution. I guess it will even re-execute if the parent re-renders for example.

I've opened another issue: #48