Warning in Unmounted Component (react-router)

[eeyang92]

Hi there, I currently have a Login Component, and based on the server's response I will call (react-router-dom) <Redirect to='/dashboard' /> if the server responds with affirmation the user has a valid session cookie already. However, I get this warning inside my component:

Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.
    in Login (created by LoginBox)
    in div (created by Card)
    in div (created by Card)
    in Card (created by LoginBox)
    in LoginBox (created by Route)

To clarify, the setState is already done by the time the redirect is called:

// Hooks, Handlers, etc.
...
if (authorized) {
        return <Redirect to='/dashboard' />
}
// else, Display Login Component
...

From the Warning, I get a hint that something should be cleaned up? Not sure if this is related to #5.

[quisido]

Can you provide a minimal example to reproduce? I'd love to get this error fixed for you if I can recreate it on my own environment.

[eeyang92]

Sure!

index.tsx:

import * as React from 'react'
import * as ReactDOM from 'react-dom'
import { BrowserRouter, Route, Redirect } from 'react-router-dom'
import { setGlobal, useGlobal } from 'reactn'

type GlobalState = {
    authorized: boolean,
    authChecked: boolean
}

const initialState: GlobalState = {
    authorized: false,
    authChecked: false
}

setGlobal(initialState)

function LoginPage() {
    const [authorized, setAuthorized] = useGlobal('authorized')
    const [authChecked, setAuthChecked] = useGlobal('authChecked')

    function onLogin(event: any) {
        event.preventDefault()

        setTimeout(() => { // Try without setTimeout
            setAuthorized(true)
            setAuthChecked(true)
        }, 500)
    }

    if (authorized) {
        return <Redirect to='/dashboard' />
    }

    return (
        <div>
            <div>This the the login page</div>
            <button onClick={ onLogin }>Login</button>
        </div>
    )
}

function DashboardPage() {
    const [authorized, setAuthorized] = useGlobal('authorized')
    const [authChecked, setAuthChecked] = useGlobal('authChecked')

    if (authChecked && !authorized) {
        return <Redirect to='/login' />
    }

    return (
        <div>This the the dashboard</div>
    )
}

function IndexPage() {
    const [authorized, setAuthorized] = useGlobal('authorized')
    const [authChecked, setAuthChecked] = useGlobal('authChecked')

    if (!authChecked) {
        return (
            <div>This is the index page!</div>
        )
    } else {
        return (
            authorized ? (<Redirect to='/dashboard' />) : (<Redirect to='/login' />)
        )
    }
}

export default function App() {
    const [authorized, setAuthorized] = useGlobal('authorized')
    const [authChecked, setAuthChecked] = useGlobal('authChecked')

    React.useEffect(() => {
        if (!authChecked) {
            setTimeout(() => {
                console.log('effect')
    
                setAuthorized(false) // set to true for auto-redirect on login page
                setAuthChecked(true)
            }, 500)
        }
    }, [authorized, authChecked])

    return (
        <BrowserRouter>
            <div>
                <Route path='/' exact component={ IndexPage } />
                <Route path='/login/' component={ LoginPage } />
                <Route path='/dashboard/' component={ DashboardPage } />
            </div>
        </BrowserRouter>
    )
}

ReactDOM.render(<App />, document.getElementById('app'))

package.json:

"dependencies": {
    "react": "16.7.0-alpha.2",
    "react-dom": "16.7.0-alpha.2",
    "react-router-dom": "^4.3.1",
    "reactn": "^0.1.6"
}

After some more investigation, I found that if you remove the setTimeout from the login button, no Warning is shown, which could suggest some interaction with the async behavior.

If you need my webpack config, I can provide that too (although I don't think it would be relevant, but you never know).

[BDQ]

I've encountered similar issues, I think there needs to be a way to remove all listeners when a component unmounts.

[quisido]

All listeners are currently supposed to be removed on unmount. I will look into this issue Sunday or Monday, as I am away for the next few days. :)

…

On Thu, Dec 6, 2018, 11:52 AM Brian Quinn ***@***.*** wrote: I've encountered similar issues, I think there needs to be a way to remove all listeners when a component unmounts. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#28 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAU_HSHaPaWVIKgHcpwjIpuE5BtErjt0ks5u2XWYgaJpZM4ZCL0f> .

[quisido]

@eeyang92 I have copied your code verbatim, but I am not getting an error.

I used create-react-app to spin up an application. I deleted everything in src and added src/index.tsx:

import * as React from 'react'
import * as ReactDOM from 'react-dom'
import { BrowserRouter, Route, Redirect } from 'react-router-dom'
import { setGlobal, useGlobal } from 'reactn'

type GlobalState = {
    authorized: boolean,
    authChecked: boolean
}

const initialState: GlobalState = {
    authorized: false,
    authChecked: false
}

setGlobal(initialState)

function LoginPage() {
    const [authorized, setAuthorized] = useGlobal('authorized')
    const [authChecked, setAuthChecked] = useGlobal('authChecked')

    function onLogin(event: any) {
        event.preventDefault()

        setTimeout(() => { // Try without setTimeout
            setAuthorized(true)
            setAuthChecked(true)
        }, 500)
    }

    if (authorized) {
        return <Redirect to='/dashboard' />
    }

    return (
        <div>
            <div>This the the login page</div>
            <button onClick={ onLogin }>Login</button>
        </div>
    )
}

function DashboardPage() {
    const [authorized, setAuthorized] = useGlobal('authorized')
    const [authChecked, setAuthChecked] = useGlobal('authChecked')

    if (authChecked && !authorized) {
        return <Redirect to='/login' />
    }

    return (
        <div>This the the dashboard</div>
    )
}

function IndexPage() {
    const [authorized, setAuthorized] = useGlobal('authorized')
    const [authChecked, setAuthChecked] = useGlobal('authChecked')

    if (!authChecked) {
        return (
            <div>This is the index page!</div>
        )
    } else {
        return (
            authorized ? (<Redirect to='/dashboard' />) : (<Redirect to='/login' />)
        )
    }
}

export default function App() {
    const [authorized, setAuthorized] = useGlobal('authorized')
    const [authChecked, setAuthChecked] = useGlobal('authChecked')

    React.useEffect(() => {
        if (!authChecked) {
            setTimeout(() => {
                console.log('effect')
    
                setAuthorized(false) // set to true for auto-redirect on login page
                setAuthChecked(true)
            }, 500)
        }
    }, [authorized, authChecked])

    return (
        <BrowserRouter>
            <div>
                <Route path='/' exact component={ IndexPage } />
                <Route path='/login/' component={ LoginPage } />
                <Route path='/dashboard/' component={ DashboardPage } />
            </div>
        </BrowserRouter>
    )
}

ReactDOM.render(<App />, document.getElementById('app'))

I had to change the ID from root to app to in public/index.html to match yours.

I didn't CRA with TypeScript by default, so I added these packages manually: @types/node, @types/react, @types/react-dom, @types/react-router-dom, react@next, react-dom@next, react-router-dom, reactn, and typescript.

yarn start, and I'm getting no error.

"This is the index page" flashes for a moment before I get "This the the login page" with a button. When I click the button, I'm presented with "This is the dashboard."

@BDQ If you can provide a minimal reproduction of when you've encountered this in error, I'm willing to take a look at it as well.

[eeyang92]

So I played around with it a bit more, and it looks like the warning only comes up when in a dev environment. (Specifically, react-dom.development.js)

Here is my example (cleaned up) as I have it on my system:

https://github.com/eeyang92/reactn-dev-error-example

[BDQ]

@CharlesStover - I've dug into the code in global-state-manager.js and I think the issue might be around addKeyListener and removeKeyListener storing _globalCallback as the listener value.

This issue occurs for me when I've got two components using to the same global key, it seems like removeKeyListener is removing the wrong listener, so the update is being forced on the wrong component.

[quisido]

EDIT: Both of these are reproducible.

Example for BDQ's scenario:

import * as React from 'react'
import * as ReactDOM from 'react-dom'
import { setGlobal, useGlobal } from 'reactn'

setGlobal({
    value: false
});

const A = () => {
    const [ value, setValue ] = useGlobal('value');
    return value.toString();
};

const B = () => {
    const [ value, setValue ] = useGlobal('value');
    return value.toString();
};

export default function App() {
    const [ value, setValue ] = useGlobal('value');
    setTimeout(() => {
        setValue(!value);
    }, 250);
    return (
        <>
            <A></A>
            {value && <B></B>}
        </>
    );
}

ReactDOM.render(<App />, document.getElementById('app'))

I'll get this fixed as soon as possible.

[eeyang92]

Awesome! 😄

[quisido]

I've got what I believe is a fix for this situation. The problem is when a parent and child both listen to the same property, the parent re-rendering causes the child to unmount (in addition to itself unmounting) while the child is still queued to re-render.

I've changed it so that unmounting a component also removes property listeners that are already queued to occur, and the error seems to have disappeared in the above examples.

Let me know if you still encounter this issue. My commit will close this Issue, so feel free to make a new one. This will be released in a few moments as ReactN 0.1.8.

[BDQ]

@CharlesStover that fixed my issue, thanks for the fix!