Bug: toggle component elements on/off cause rendering / virtual diff problems

[sixtram]

The following sample creates a component (GridComponent), which creates two rows (RowComponents), each have a button. Clicking the button increases a global counter and calls m.render().

During render if the render count is odd/even we want to toggle the 2nd row on and off. However mithril fails to render the second row between the >>>>> and <<<<< div elements. At the same time it correctly renders the other elements.

---

<script type="text/javascript">
    var counter = 1;

    var RowComponent = {
        controller: function () {
            console.log("RowComponent.controller() called");
            return {};
        },
        view: function (controller, args) {
            console.log("RowComponent.view() called");
            return m("div", [
                "I'm a row element. My name is " + args.fullName + " ",
                m("button", {
                    onclick: function () {
                        counter++;
                        rerender();
                    }
                }, "Increase and toggle and rerender")]);
        }
    };

    var GridComponent = {
        controller: function (args) {
            console.log("GridComponent.controller() called");
            return {};
        },
        view: function (ctrl, args) {
            console.log("GridComponent.view() called");
            var rows = [];

            rows.push(m("div", "First row"));

            // Add first row.
            rows.push(m.component(RowComponent, {
                key: 100, fullName: "1st"
            }));

            // Show/hide second row based on counter.
            if (counter % 2 == 1) {
                rows.push(m("div", ">>>>>"));
                // ******** this is problematic *********
                rows.push(m.component(RowComponent, {
                    key: 200, fullName: "2nd"
                }));
                rows.push(m("div", "<<<<<"));
            }

            rows.push(m("div", "Last row"));

            var items = [];
            items.push(m("div", "This is grid named " + args.gridTitle + ". Rendered " + counter.toString(10) + " times."));
            var allelements = items.concat(rows);
            console.log(allelements);

            return m("div", allelements);
        }
    };

    var mainGridComponent = m.component(GridComponent, { gridTitle: "Toggler Test" });

    function rerender() {
        m.render(document.getElementById("placeholder"), mainGridComponent);
    }
</script>

<div id="placeholder" />

<script type="text/javascript">
    // Do the initial render.
    rerender();
</script>

[sixtram]

op here. it seems that while the additional component is created correctly, it's view() is not called correctly.

rendering the main component two times fixes the problem, however the rendering function is a pure function.

this fixes the problem temporarily:

function rerender() { m.render(document.getElementById("placeholder"), mainGridComponent); m.render(document.getElementById("placeholder"), mainGridComponent); }

[sixtram]

op again. it seems that the issue is that you cannot call m.render() during in an event handler code, in that case newly created components are not created correctly. however if you call render later, those will be fine.

[MattMcFarland]

Could it be due to calling writable methods in view()? I was thinking that view() operations are immutable. So the best time to push new values to an array, might be either in the controller or in a separate event handler. I think that the view() method should only render items, either by using the map function or using ternaries.

[pdfernhout]

You could try calling your original rerender via something like: "setTimeout(rerender, 0);" to see if that changes anything. That would defer the rerender until the onclick event was finished processing. I tried that in this fiddle and it worked for me: https://jsfiddle.net/pdfernhout/tt7o2jjd/1/

That said, I've been calling m.render from button clicks before in a Mithril app that did not use m.mount or m.route and it seemed to be working. I'd suspect there is some other issue here, perhaps related to component nesting? Update: I've realized as I commented further below, that when I've called m.render this way using Mithril 0.2.0, there were no components involved.

[pdfernhout]

As a test to get another data point, I set the key of the second component to "200 + counter" with your original (non-timeout) render, but that did not fix the issue: https://jsfiddle.net/pdfernhout/tt7o2jjd/4/

[pdfernhout]

It turns out, if you look for the missing component in the generated HTML, it is rendered as:

which is invisible. I've seen something like that before unexpectedly a couple weeks ago. I think I used setTimeout to work around it. **Update: No, that is incorrect; now that I think more about it more, I recall that after seeing that issue, I had decided to just use m.mount and then everything worked.** The Mithril docs say that placeholder can be inserted for asynchronous calls during component construction. From the [m.component](https://lhorie.github.io/mithril/mithril.component.html#nested-asynchronous-components) docs: "If a component A contains another component B that calls asynchronous services, when component A is rendered, a tag is rendered in place of component B until B's asynchronous services resolve. Once resolved, the placeholder is replaced with component B's view." But this call is not asynchronous. Is it a possible bug in Mithril? Or is the documentation perhaps incomplete on this? Here is the related line of code from the build function for an OBJECT in Mithril 0.2.0: ``` data = pendingRequests == 0 || (cached && cached.controllers && cached.controllers.indexOf(controller) > -1) ? data.view(controller) : {tag: "placeholder"} ```

[pdfernhout]

I've been debugging into this code, and I think the issue may be due to Mithril wrapping all "on" events with autoredraw which calls startComputation before doing the actual callback and endComputation afterwards. I had not realized Mithril does this even if you have not mounted anything. And apparently there is no way to not have these callbacks wrapped -- which means some extra calculations even if you manually call m.render yourself whenever needed (as I do for one page).

The global variable pendingRequests is incremented in startComputation. So, in this line of code that sets data, Mithril won't just always return the result of data.view(controller), and there won't be a cached controller otherwise when the component is being created. And if nothing is mounted, when endComputation is called, nothing will the be redrawn.

I don't know if I could call this a bug then as much as a design limitation perhaps in a system apparently designed primarily for being used with m.mount and m.route?

However, I don't really understand the reason for this use of pendingRequests this way in view functions. I wonder if the global pendingRequests should be initialize to -1 instead of 0 in the library? Then when you start the very first computation, there won't be any "pendingRequests". Alternatively, and maybe better, the test could be for "pendingRequests <= 1" with the assumption the first request is at the top level?

Using setTimeout to queue the m.render call gets around this situation for any code that has components.

I just searched for "mithril" and "pendingRequests", and I find issue #603 which is related, although it is about "m.redraw" and not "m.render". Leo said a fix for that issue was pushed to the "next" branch. Looking at the next branch, I see that line of code now reads:

data = (pendingRequests == 0 || forcing) || (cached && cached.controllers && cached.controllers.indexOf(controller) > -1) ? data.view(controller) : {tag: "placeholder"};

The global variable forcing is set to true in redraw but not render. So, I don't see how that change will fix the m.render aspect of what may be a common issue unless there are different changes also made. Trying to test the code with Mithril next as of today, this test indeed still does not work.

If I change that test to use "pendingRequests <= 1" instead of "pendingRequests == 0", the test works. I'm not sure if that is an OK solution though. One question is, what does a "pendingRequests" of 1 really mean?

I'm wondering if Mithril itself should use setTimeout to queue any redraws (with debouncing) to avoid this sort of recursion issue of a redraw called from inside a handler? Could that make the code simpler in this case? Then the render function could be more straightforward perhaps, without worrying about "forcing"? In that case, m.startComputation and m.endComputation could still be used by third-party libraries, but endComputation would then call m.redraw when done which would in turn call setTimeout (with debouncing so pending redraws were coalesced, and maybe an optional argument for debugging purposes to explain who called redraw). It seems to me that "asynchronous" components could perhaps be implemented by just rendering their own placeholder text (like "Loading...") rather than having this extra layer which leads to this sort of unexpected behavior in the synchronous case? I think that issue has been mentioned on the mailing list? However, I have not used m.request or asynchronous components, so there may be other issues I'm not aware of.

Until that possibility or some other alternative is thought through and implemented, using setTimeout seems like a good workaround for this m.render issue if you want to mix using m.render with having nested components (and so avoiding m.mount).

[tivac]

Closing due to lack of movement.