Skip to content

Clarification and Potential Improvement on PR #480 useEffectOnce behavior #595

Open
@lukaszkrzywizna

Description

@lukaszkrzywizna

@Zaid-Ajaj @JordanMarr I've checked the PR #480 and I wonder if that code is correct when not using strict mode:

if renderAfterCalled.current
then destroyFunc.current
else None

I'm asking because if I an component unmounts the destroy func is not invoked:

// not using strict mode

let dispose = React.createDisposable(fun () -> eprintf "Disposed")
    
// dispose never invoked
React.useEffectOnce(fun () -> dispose)
    
// disposed invoked
React.useEffect((fun () -> dispose), [||])

I wonder what's the point of this function? If it's created to handle strick mode's double useEffect run, then we should reflect this in a name or comment. Otherwise we should fix it somehow - my first idea is to add check for a debug:

  static member private useEffectOnce(effect: unit -> System.IDisposable) = 
    let destroyFunc = React.useRef None
    let calledOnce = React.useRef false
    let renderAfterCalled = React.useRef false

    if calledOnce.current then
        renderAfterCalled.current <- true
    
    React.useEffect (fun () -> 
        if calledOnce.current 
        then None
        else
            calledOnce.current <- true
            destroyFunc.current <- effect() |> Some
#if DEBUG
            if renderAfterCalled.current
#else
            if not renderAfterCalled.current
#endif
            then destroyFunc.current
            else None
    , [||])

But this does not guarantee that user has strict mode enabled and we can still have situation with not invoked dispose.

What do you think about it?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions