Model references are not resolved when trying to change ranges and attach JS callback at the same time

[p-himik]

Bokeh: 0.12.6.

Example code:

from bokeh.plotting import figure, curdoc
from bokeh.models import Range1d, CustomJS, Button
from bokeh.layouts import column

f = figure(width=500, height=500,
           x_range=Range1d(start=0, end=1),
           y_range=Range1d(start=0, end=1))


def callback():
    f.x_range.start = 0
    f.x_range.end = 1
    f.y_range.start = 0
    f.y_range.end = 1
    cb = CustomJS(args=dict(x_range=f.x_range, y_range=f.y_range),
                  code="""
                    console.log(x_range.plots);
                    if (x_range.plots === undefined) {
                        alert('plots are undefined!');
                    }
                    """)
    f.x_range.callback = cb
    f.y_range.callback = cb


button = Button(label="Test")
button.on_click(callback)

curdoc().add_root(column(button, f))

Run with bokeh serve, open in your browser and press "Test" button repeatedly - after some attempts you should get an alert message.

The issue is that the WS messages that are being sent as the result of this callback execution contain references field with an array that can contain objects in different order. If Range1d goes before CustomJS - all is fine and you don't see the error. If Range1d is after CustomJS, the references for CustomJS are not resolved as Bokeh thinks that CustomJS is an existing object.

Take a look at this debug session screenshot (it's taken with a different JS code in the callback, but it doesn't matter):

In the code section, it's clearly seen that the latest value of was_new must have come from ref1[2]. However, in the Scope section ref1[2] is true (as it should be for the new callback object), but was_new is false.
I have not a single clue on why it happens, but apparently, was_new value was not set or was taken from the outer closure instead. The issue is reminiscent of asynchronous callbacks that depend on the iteration variable, but in this case, there are no asynchronous calls.

[mattpap]

This is beautiful and exactly why we are switching to typescript. The problem is that was_new isn't a local variable, because it was already declared outside foreach_value() and foreach_depth_first(), and is pretty much a global variable at this point. Between setting was_new and using it in foreach_value(), it's being overwritten by next iterations. Good news is that I'm rewriting src/coffee/document.coffee in typescript right now, so this will be fixed soon.

[mattpap]

@p-himik, btw., this is the kind of bug report I appreciate. Everything is here to figure it out. No need for me to even open a code editor or a web browser.

[p-himik]

@mattpap Wow, now I feel stupid. :) Looked at the code so much that I completely missed the recursive call that changes global var. Thanks for the explanation.

[mattpap]

This was fixed in PR #7233.

[p-himik]

@mattpap Was it though? The root cause of the issue, the global reference to was_new, is still there in the compiled JS.

[mattpap]

(...) the global reference to was_new, is still there in the compiled JS.

Sure, but the inner was_new is now var prefixed (within the inner function scope). In all honesty, I didn't test the code in this issue, but on theoretical grounds this should work correctly now.

[p-himik]

Is it possible that we're getting different compiled document.js files?

There's only one was_new declaration. The screenshot doesn't include the whole function, but the rest of the text doesn't contain other was_new declarations.

[mattpap]

@p-himik, are you sure you have most recent master? This is definitively obsolete output (from coffeescript). The PR that fixed this was merged just a few days ago.

[p-himik]

Sorry, you're right - I saw document.ts in my working copy of and assumed that it was that.