Subscribed Issues and Rendering

darth-cheney commented 4 years ago

So I've been wracking my brain around the "three-pane toggle" bug that we identified at the end of last week. I have a fairly concrete idea of what the issue is (discussed below), but the several attempts at finding a reasonably simple solution have all failed. So I'm at a point where I think I should run it past you guys.

The Problem

First, recall what is happening. There is a new example, both on brax-dev and my recently pushed branch eric-subscribed-issues under the resizable_panel demos. You will see that if you toggle any one thing on then off, it all works fine. But if you toggle off the first pane, then either the second or third, then try to once more toggle (on) the first pane, the browser will error out.

At the Cells level, here is why that is happening. Look at this part of the code:

return toggles() + cells.Flex(
            cells.Subscribed(
                lambda: cells.ResizablePanel(
                    cells.Subscribed(firstDisplay),
                    cells.Subscribed(toggledSecondAndThird),
                    ratio=0.20,
                )
                if ss.showFirst
                else cells.Subscribed(toggledSecondAndThird)
            )
        )

The structure of this description is that when all panes are "on," there should be a Subscribed wrapping a ResizablePanel that itself contains two Subscribeds, the second of which might itself be another nested ResizablePanel (for panes 2 and 3 if both are enabled). Depending on the configuration, a pane might be rendered inside of a Panel or a ResizablePanel. It can change, and for reasons I'll get to in a second that's important.

But just looking again at this snippet you can see a problem right away: inside of the outer Subscribed we will either render:

A ResizablePanel that has two child Subscribeds
A nested Subscribed that has either a ResizablePanel or just a Pane or nothing.

Once we have the ResizablePanel, and then it goes away and is replaced by two immediately nested Subscribeds, when we try to add it back in the frontend has no way of knowing where it should go anymore.

Here is how Subscribed works on the frontend. Any Subscribed component will do an initial build() on its content child, and then add the data-subscribed-to=<Subscribed.id> to the child content that actually gets rendered. Later, when its time to update that Subscribed, it looks up its last entrypoint in the DOM by querying for the element that has the matching data-subscribed-to data member.

Recall that in this example we first render a ResizablePanel, but then it goes out of the DOM. Now when trying to toggle it back on there is no element that has data-subscribed-to=<Subscribed.id> -- that would have been the ResizablePanel.

What I've Tried

I briefly tried reversing to the "old way" of simply rendering Subscribed components as plain div elements where needed in the rendering tree. This predictably didn't work -- it breaks styling and layout all over the place and leads to unpredictable visual results (this is why we stopped doing it in the first place).

I also attempted to rewire how the Subscribed component "knows" where to render its contents, primarily by laying down a placeholder for itself whenever the content is empty so it can come back and re-insert the real thing there. The problem with this methods occurs when you have a direct parent-child coupling of two Subscribed components (as we do in the offending example here). Making exceptions for that led to too much complexity and unexpected behavior.

What Now

I am starting to run out of ideas, but I have one more that might work. I need your input on it. It hinges on the following question: Why do we need to send Subscribed over the wire at all and why does it need to be a Component? I think we should be able to determine on the Python side whether or not a Subscribed's content has changed (and therefore its parent description needs to be updated over the wire), rather than doing anything on the frontend. I think we already have a semi-workable framework making this possible, where the "true" parent is different from the "display" parent.

Do you think that could work? Am I missing something that might make this approach unfeasible?

braxtonmckee commented 4 years ago

I think that sounds reasonable. What you're basically saying is that we'll maintain the tree on the python side, but when we send a representation to the frontend it will have no Subscribeds in it all. To make this work, though, you'll need to be able to apply changes differences to children of any cell type. For instance, in the current framework, a ResizeablePanel doesn't ever expect one of its children to 'change' because that child would always be a 'subscribed'. I don't know how big of a change on the frontend that is.

On Thu, Dec 12, 2019 at 1:25 PM Eric Gade notifications@github.com wrote:

So I've been wracking my brain around the "three-pane toggle" bug that we identified at the end of last week. I have a fairly concrete idea of what the issue is (discussed below), but the several attempts at finding a reasonably simple solution have all failed. So I'm at a point where I think I should run it past you guys. The Problem

First, recall what is happening. There is a new example, both on brax-dev and my recently pushed branch eric-subscribed-issues under the resizable_panel demos https://github.com/APrioriInvestments/object_database/blob/eric-subscribed-issues/object_database/web/cells_demo/resizable_panel.py#L63. You will see that if you toggle any one thing on then off, it all works fine. But if you toggle off the first pane, then either the second or third, then try to once more toggle (on) the first pane, the browser will error out.

At the Cells level, here is why that is happening. Look at this part of the code:

return toggles() + cells.Flex( cells.Subscribed( lambda: cells.ResizablePanel( cells.Subscribed(firstDisplay), cells.Subscribed(toggledSecondAndThird), ratio=0.20, ) if ss.showFirst else cells.Subscribed(toggledSecondAndThird) ) )

The structure of this description is that when all panes are "on," there should be a Subscribed wrapping a ResizablePanel that itself contains two Subscribeds, the second of which might itself be another nested ResizablePanel (for panes 2 and 3 if both are enabled). Depending on the configuration, a pane might be rendered inside of a Panel or a ResizablePanel. It can change, and for reasons I'll get to in a second that's important.

But just looking again at this snippet you can see a problem right away: inside of the outer Subscribed we will either render:

A ResizablePanel that has two child Subscribeds

A nested Subscribed that has either a ResizablePanel or just a Pane or nothing.

Once we have the ResizablePanel, and then it goes away and is replaced by two immediately nested Subscribeds, when we try to add it back in the frontend has no way of knowing where it should go anymore.

Here is how Subscribed works on the frontend. Any Subscribed component will do an initial build() on its content child, and then add the data-subscribed-to= to the child content that actually gets rendered. Later, when its time to update that Subscribed, it looks up its last entrypoint in the DOM by querying for the element that has the matching data-subscribed-to data member.

Recall that in this example we first render a ResizablePanel, but then it goes out of the DOM. Now when trying to toggle it back on there is no element that has data-subscribed-to= -- that would have been the ResizablePanel. What I've Tried

I briefly tried reversing to the "old way" of simply rendering Subscribed components as plain div elements where needed in the rendering tree. This predictably didn't work -- it breaks styling and layout all over the place and leads to unpredictable visual results (this is why we stopped doing it in the first place).

I also attempted to rewire how the Subscribed component "knows" where to render its contents, primarily by laying down a placeholder for itself whenever the content is empty so it can come back and re-insert the real thing there. The problem with this methods occurs when you have a direct parent-child coupling of two Subscribed components (as we do in the offending example here). Making exceptions for that led to too much complexity and unexpected behavior. What Now

I am starting to run out of ideas, but I have one more that might work. I need your input on it. It hinges on the following question: Why do we need to send Subscribed over the wire at all and why does it need to be a Component? I think we should be able to determine on the Python side whether or not a Subscribed's content has changed (and therefore its parent description needs to be updated over the wire), rather than doing anything on the frontend. I think we already have a semi-workable framework making this possible, where the "true" parent is different from the "display" parent.

Do you think that could work? Am I missing something that might make this approach unfeasible?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/APrioriInvestments/object_database/issues/62?email_source=notifications&email_token=AB6OHBACKLHFXIUYDRJFSKLQYJ63HA5CNFSM4J2CURGKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IAEEE4Q, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB6OHBHEXALSUQRLFUF67E3QYJ63HANCNFSM4J2CURGA .

darth-cheney commented 4 years ago

What you're basically saying is that we'll maintain the tree on the python side, but when we send a representation to the frontend it will have no Subscribeds in it all.

Right, because on the frontend it doesn't matter. We only care about the visual tree and the relationships that matter for display. It already assumes that any of it can change at any time.

For instance, in the current framework, a ResizeablePanel doesn't ever expect one of its children to 'change' because that child would always be a 'subscribed'.

Can't we just handle this case in the "recalculate cells" portion of the cycle? We only care about the Subscribed itself when its first added or ultimately removed. For anything in the interim we only care about the Subscribed's contents (and what the Subscribed's parent is), correct?

I don't know how big of a change on the frontend that is.

There wouldn't be much of a change at all, so long as we are sending over correct create/update/destroy messages that don't include anything about Subscribeds (unless I'm missing something)

darth-cheney commented 4 years ago

@braxtonmckee FYI, I've just submitted a PR that seems to deal with this problem: https://github.com/APrioriInvestments/object_database/pull/63

darth-cheney commented 4 years ago

Closed by #63

APrioriInvestments / object_database