parent's componentDidUpdate trigger before child's componentDidMount

[gogoyqj]

jsx like below:

<scroller>
    <Lazyimage />
</scroller>

preact if 'rerender' called

[email protected]:3876     Scroller     componentDidMount
[email protected]:3920     Scroller     componentDidUpdate
[email protected]:5633     Lazyimage  componentDidMount

In react

[email protected]:3876     Scroller     componentDidMount
[email protected]:5633     Lazyimage  componentDidMount
[email protected]:3920     Scroller     componentDidUpdate

the difference in trigger order may cause problem

[developit]

Ah yeah that makes sense, since Preact defers componentDidMount until the very end of the cycle.

[gogoyqj]

seems that business logic should avoid depending on unpredictable trigger order is the best practice.

I'll change my scroller & Lazyimage

[developit]

That'd be ideal yeah - mind cc'ing me on your change or sending me a link? I like to keep on top of these things 👍

[gogoyqj]

class Scroller extends Component {

    constructor() {
        this.lazyImgs = [];
    }

    componentDidMount() {
        this._loadImg();
    }

    componentDidUpdate() {
        this._loadImg();
    }

    _loadImg() {
        if (this.lazyImgs.length) {
            this.lazyImgs.map(img => img.load());
        }
    }

}


class Lazyimage extends Component {

    componentDidMount() {
        this.scroller = this.props.scroller;
        if (this.scroller) {
            this.scroller.lazyImgs.push(this)
        }
        // here my change
        if (InView(this)) this.load()
    }

    load() {
        // load self
    }
}

I don't understand why they make the Scroller & Lazyimage work strictly depends on trigger order of parent's componentDidUpdate & children's componentDidMount.

In order to make Scroller & Lazyimage work also in preact, Lazyimage should make a decision whether to load itself when its componentDidMount trigger.

@developit that's all.

[NekR]

@developit so that https://github.com/developit/preact/pull/506 is supposed to fix this?

[Vija02]

Just to add, I found a problem caused by this today. Was using react-bootstrap's modal and apparently it renders a component that creates an element on its componentDidUpdate. But since the order of the triggered lifecycle is different, everything just broke.

So yeah, would be nice to get this fixed :smile:

[developit]

Ah interesting, that might make this the root cause of maybe 4 different issues.

[gogoyqj]

i wonder if changes thoses functions to ensure lifecycle in right order.

is there any side effect?

is there any thing that i neglect?

@developit

// in vdom/diff.js
export function flushMounts() {
    let c;
    while ((c=mounts.pop())) {
        // if (options.afterMount) options.afterMount(c);
        // if (c.componentDidMount) c.componentDidMount();
        if (typeof c === 'function') c();
    }
}

    // in vdom/component.js
    if (!isUpdate || mountAll) {
        // mounts.unshift(component);
        if (options.afterMount) mounts.unshift(() => options.afterMount(component));
        if (component.componentDidMount) mounts.unshift(component.componentDidMount.bind(component));
    }
    else if (!skip) {
        // if (component.componentDidUpdate) {
        //  component.componentDidUpdate(previousProps, previousState, previousContext);
        // }
        // if (options.afterUpdate) options.afterUpdate(component);
        if (options.afterUpdate) mounts.unshift(() => options.afterUpdate(component));
        if (component.componentDidUpdate) mounts.unshift(component.componentDidUpdate.bind(component, previousProps, previousState, previousContext));
    }

[developit]

@gogoyqj that would create a closure on every call to renderComponent(), which is very very often. Generally we try to avoid ever creating closures (I don't think there are any currently in the codebase).