Skip to content
This repository has been archived by the owner on Dec 31, 2020. It is now read-only.

return wrong componentDidMount function in mixins.real #581

Closed
kudinov-k opened this issue Oct 19, 2018 · 11 comments
Closed

return wrong componentDidMount function in mixins.real #581

kudinov-k opened this issue Oct 19, 2018 · 11 comments

Comments

@kudinov-k
Copy link

kudinov-k commented Oct 19, 2018

return target[mobxMixins][methodName]

I have 2 components (A, B) both have method componentDidMount. When A.render() is complete then runs B.componentDidMount() instead A.componentDidMount(), even I have only component A on page

@mweststrate
Copy link
Member

cc @xaviergonz

@xaviergonz
Copy link
Contributor

I'm sorry but I don't quite get what you mean, could you share a piece of code or something?
Also, what's the error you get?

@xaviergonz
Copy link
Contributor

xaviergonz commented Oct 19, 2018

I just tried this unit test and it was ok (assuming this is what you meant)

it("componentDidMount should be different between components", async () => {
    async function test(withObserver) {
        const events = []

        class A extends React.Component {
            componentDidMount() {
                this.didMount = "A"
                events.push("mountA")
            }

            componentWillUnmount() {
                this.willUnmount = "A"
                events.push("unmountA")
            }

            render() {
                return null
            }
        }

        class B extends React.Component {
            componentDidMount() {
                this.didMount = "B"
                events.push("mountB")
            }

            componentWillUnmount() {
                this.willUnmount = "B"
                events.push("unmountB")
            }

            render() {
                return null
            }
        }

        if (withObserver) {
            A = observer(A)
            B = observer(B)
        }

        const aRef = React.createRef()
        await asyncReactDOMRender(<A ref={aRef} />, testRoot)
        const caRef = aRef.current

        expect(caRef.didMount).toBe("A")
        expect(caRef.willUnmount).toBeUndefined()
        expect(events).toEqual(["mountA"])

        const bRef = React.createRef()
        await asyncReactDOMRender(<B ref={bRef} />, testRoot)
        const cbRef = bRef.current

        expect(caRef.didMount).toBe("A")
        expect(caRef.willUnmount).toBe("A")

        expect(cbRef.didMount).toBe("B")
        expect(cbRef.willUnmount).toBeUndefined()
        expect(events).toEqual(["mountA", "unmountA", "mountB"])

        await asyncReactDOMRender(null, testRoot)

        expect(caRef.didMount).toBe("A")
        expect(caRef.willUnmount).toBe("A")

        expect(cbRef.didMount).toBe("B")
        expect(cbRef.willUnmount).toBe("B")
        expect(events).toEqual(["mountA", "unmountA", "mountB", "unmountB"])
    }

    await test(true)
    await test(false)
})

@kudinov-k
Copy link
Author

I have a base class A and 3 classes (B, C, D) extended of A. A extends of React.component. B C D - 3 pages of App and they have componentDidMount. When component B renders, after that triggers C.componentDidMount. If I remove this one, than trigers D.componentDidMount and if remove it, only then will be triggered B.componentDidMount. All 4 classes has @observer annotaion. I hope it's more clearly)

@xaviergonz
Copy link
Contributor

Made a unit test and a PR, hopefully that should fix it

@skiritsis
Copy link
Contributor

skiritsis commented Oct 22, 2018

We face the same issue as @kudinov-k. Reverting back to "5.2.8" seems to fix it.

@kricore
Copy link

kricore commented Oct 22, 2018

+1 this patch did the trick. Thanks :)

@mweststrate
Copy link
Member

@skiritsis on what version?

@skiritsis
Copy link
Contributor

skiritsis commented Oct 22, 2018

@mweststrate We updated today to version 5.3.4 and just noticed it. So reverting back to the previous version we had(5.2.8) works as expected. This seems like a regression issue between those 2 versions. Let me know if you need any repro examples.

@xaviergonz
Copy link
Contributor

@mweststrate there's a fix on a pr pending :)

@mweststrate
Copy link
Member

Released as 5.3.5!

@github-actions github-actions bot mentioned this issue Oct 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants