Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue-1213 clientside callback_context Initial Commit #1240

Merged
merged 14 commits into from
May 14, 2020

Conversation

carl-dawson
Copy link

@carl-dawson carl-dawson commented May 10, 2020

Clientside callback_context

Here’s a first pass at Issue-1213, adding callback_context to clientside callbacks. Before I get too much further into the weeds, I wanted to get a PR going to discuss.

The idea was to mimic the behavior of the serverside dash.callback_context as closely as possible. I think this is probably the way to go for consistency’s sake, but I am of course open to suggestions.
From within a clientside callback function, we can now use the trick with “triggered” to figure out which input triggered the callback:

triggered = dash_clientside.callback_context.triggered.map(t => t[“prop_id”]);

The first new test test_clsd009_clientside_callback_context is currently passing.

Contributor Checklist

  • TODO
    • Add property states
    • Add property states_list
    • Add property response (need to check if this even makes sense in the clientside context)
    • Flush out tests for each of the context properties
  • All tests passing

Two quick off-topic things:

  • I get the following warning when setting up pip install -e .[testing,dev]:

requests 2.21.0 has requirement urllib3<1.25,>=1.21.1, but you'll have urllib3 1.25.9 which is incompatible.

  • I had to comment out all of /test/integration/test_generation.py to gets pytest to work.
    I’m on Windows, Python 3.7

@alexcjohnson
Copy link
Collaborator

@MrDawson this looks great! Just a few comments:

Rather than checking for its existence, I'd just set up dc.callback_context before the callback is executed, then delete it when the callback is done. And if we do that, I'm not even sure it's necessary to use Object.defineProperty - the only reason to do that with no_update and PreventUpdate was so they wouldn't be accidentally overwritten, but that wouldn't be a concern anymore.

All tests passing

npm run format from the dash-renderer dir can fix the JS lint issues, and for the Py2 test unfortunately we'll have to avoid f-strings. Yes, we still have some users on Py2...

requests 2.21.0 has requirement urllib3<1.25,>=1.21.1, but you'll have urllib3 1.25.9 which is incompatible.

Looks like our requests requirement is out of date. We do update them fairly regularly but if this causes any problems the safest thing to do is run tests in a venv.

I had to comment out all of /test/integration/test_generation.py to gets pytest to work.
I’m on Windows, Python 3.7

Hmm, good to know! None of our core team works on Windows normally but this is of course an important environment to support. We do have a Windows test run on CI build-windows-37, which passes, but perhaps we should be running a few more of our tests in that environment. Would you mind creating a new issue for this and detailing the errors you saw?

@carl-dawson
Copy link
Author

  • The integration tests were all working on my end (Py 2.7 and 3.7). Anyone know why they are failing on CirleCI?
  • response is not included at this point. If someone wants to clarify why/how to add it, I can do so.
  • @alexcjohnson Why delete the dc.callback_context after the callback is executed? Is it just best practice to free up the space?

@alexcjohnson
Copy link
Collaborator

The integration tests were all working on my end (Py 2.7 and 3.7). Anyone know why they are failing on CirleCI?

Yeah we really need a better error message from wait_for_text_to_equal and its friends, to show what the text is, not just what it isn't... My guess would be somehow the browser is different and Object.keys or JSON.stringify picked the keys in a different order. Object.keys you can just sort, JSON.stringify is trickier but here's one that should suffice:

function stringifySorted(v) {
    if (Array.isArray(v)) {
        return '[' + v.map(stringifySorted).join(',') + ']';
    }
    if (v && typeof v === 'object') {
        const parts = Object.keys(v)
            .sort()
            .map(k => JSON.stringify(k) + ':' + stringifySorted(v[k]));
        return '{' + parts.join(',') + '}';
    }
    return JSON.stringify(v);
}

If that doesn't work, in order to debug you can just put in a few seconds sleep and do assert dash_duo.get_element('#your-id').text == expected - which will show the actual string found in the error message.

response is not included at this point. If someone wants to clarify why/how to add it, I can do so.

Not useful for client-side callbacks. This is mostly to support server-side callbacks setting cookies.

Why delete the dc.callback_context after the callback is executed? Is it just best practice to free up the space?

Maybe not super important, but we do this server side so that if someone tries to use callback_context when they're not in a callback it will fail rather than give them stale, meaningless data.

@alexcjohnson
Copy link
Collaborator

Love it! Thanks for the changes, and your tests are fantastic. Glad to see they work without a fancy custom stringify after all 🎉

The only last thing I'd ask for is a changelog entry. Just add ## [UNRELEASED] at the top and describe this feature under ### Added. Then we'll be ready to merge!

@carl-dawson
Copy link
Author

🎉Thanks for your quick feedback!

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thanks again @MrDawson! 💃

@BaconEggsRL
Copy link

BaconEggsRL commented Jan 6, 2024

Hi, beginner here having trouble getting this to work.

I get "undefined" for 'window.dash_clientside.callback_context;'
'window.dash_clientside' is defined but has no property 'callback_context'

Thanks

@Coding-with-Adam
Copy link
Contributor

hi @BaconEggsRL
Can you please share more information on what's not working?
Can you share the code that is creating this error?

@BaconEggsRL
Copy link

BaconEggsRL commented Jan 9, 2024

I have similar issue as these folks:
https://community.plotly.com/t/how-to-use-two-functions-for-clientside-callbacks-within-js-files/71084/7
https://community.plotly.com/t/clientside-callback-example-with-js-function-throws-cannot-read-property-apply-of-undefined-error/44411/5

I can only get the function to work as an inline function. When I try to put it in .js file I get undefined error.

Error:

TypeError: Cannot read properties of undefined (reading 'test_function')

Version

dash                 2.14.2
dash-ag-grid         2.4.0

assets\dashAgGridComponentFunctions.js

window.dash_clientside = Object.assign({}, window.dash_clientside, {
    clientside: {
        test_function: function(btn-1, btn-2) {
            return "Hello from someone"
        }
    }
});

test_clientside_callback.py

from dash import Dash, html, dcc, Output, Input, clientside_callback, ClientsideFunction

app = Dash(__name__)

app.layout = html.Div(
    [
        dcc.Markdown(f"Log (inline function)"),
        html.Div(id='log-clientside-inline', style={'display': 'block'}),  # none | block

        dcc.Markdown(f"Log (dashAgGridComponentFunctions.js function)"),
        html.Div(id='log-clientside-js', style={'display': 'block'}),  # none | block

        html.Button('btn-1', id="btn-1"),
        html.Button('btn-2', id="btn-2"),
    ]
)

# Clientside callback context (inline)
clientside_callback(
    """
    function(btn1, btn2) {
        // console.log('inline test')
        const triggered = dash_clientside.callback_context.triggered.map(t => t.prop_id);
        return "Hello from [" + triggered + "]"
    }
    """,
    Output("log-clientside-inline", "children"),
    Input("btn-1", "n_clicks"),
    Input("btn-2", "n_clicks"),
    prevent_initial_call=True,
)

# Clientside callback context (.js)
clientside_callback(
    ClientsideFunction(
        namespace='clientside',
        function_name='test_function'
    ),
    Output('log-clientside-js', 'children'),
    Input('btn-1', 'n_clicks'),
    Input('btn-2', 'n_clicks'),
    prevent_initial_call=True,
)


if __name__ == "__main__":
    app.run(debug=True)

@BaconEggsRL
Copy link

BaconEggsRL commented Jan 9, 2024

@Coding-with-Adam I think I fixed it just now. I don't think there is any issue with dash just some simple mistakes.

Two issues:

  • 1st issue: Apparently JavaScript variables cannot have "-" characters. So that caused an error. I'm not familiar with JavaScript and all the example ids have dashes in them so I didn't think anything of it. Not sure if in the docs somewhere but might be worth mentioning.

  • 2nd issue: I moved my test scripts into another folder inside the main project directory. So they can no longer see the assets folder or any of the functions. I created a new assets folder for the test scripts and that solved the issue.

It does not seem to matter the name of the .js file as long as it is in the assets folder.
So the filestructure must be like this:

folder/test.py
folder/assets/whatever_name_you_want.js

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants