dash icon indicating copy to clipboard operation
dash copied to clipboard

[BUG] Dash Design Kit's ddk.Notification does not render correctly on React 18.2.0

Open rymndhng opened this issue 1 year ago • 6 comments

Describe your context Please provide us your environment, so we can easily reproduce the issue.

  • replace the result of pip list | grep dash below
dash                      2.9.3
dash-core-components      2.0.0
dash-html-components      2.0.0
dash-table                5.0.0
dash_cytoscape            0.2.0
  • if frontend related, tell us your Browser, Version and OS

    • OS: MacOS[e.g. iOS]
    • Browser Firefox, Chrome
    • Version [e.g. 22]

Describe the bug

ddk.Notification rendering is inconsistent. It does not render immediately until the next UI event. This is incredibly buggy on React 18.

Reproduction Steps on this Example:

  1. Click on "Click Me"
  2. Observe that "was inserted!" is added to the DOM, but the ddk.Notification does not show up.
import dash
import dash_design_kit as ddk
from dash import Dash, dcc, html, Input, Output

app = Dash(__name__)

# Enable react 18
# See https://github.com/plotly/dash/pull/2260/files
dash._dash_renderer._set_react_version("18.2.0")

app.layout = ddk.App(
    children=[
        ddk.Header(ddk.Title("Hi")),
        html.H1(children="Hello Dash"),
        html.Button(id="click", children="Click Me!"),
        html.Div(id="stuff"),
    ]
)


@app.callback(
    Output("stuff", "children"), Input("click", "n_clicks"), prevent_initial_call=True
)
def insert_notification(n_clicks):
    return html.Div(
        children=[
            html.Div("was inserted!"),
            ddk.Notification(
                type="danger",
                title=f"n_clicks: {n_clicks}",
                timeout=-1,
            ),
        ]
    )


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

Expected behavior

It renders immediately on each key press.

Screenshots

I've included a screencapture of this behavior comparing React 16 and React 18.

React 16: https://user-images.githubusercontent.com/1694040/235269740-57d35c94-530e-432f-b052-0b7bf7de4302.mov

React 18: https://user-images.githubusercontent.com/1694040/235269470-159ee33b-994a-4ba3-a5a2-ae42eff829a5.mov

rymndhng avatar Apr 28 '23 23:04 rymndhng

Dash uses React 16, ticket to upgrade it to React 18.2 has not moved in a while. dash._dash_renderer._set_react_version("18.2.0") is experimental. If you need this simple functionality in React 18, I would suggest creating a React app, it's much easier than you think. :)

Bidek56 avatar May 16 '23 18:05 Bidek56

Hi there!!! Your code uses the dash._dash_renderer._set_react_version("18.2.0") function to enable React 18.2.0 in Dash. However, it should be noted that this feature is experimental and not officially supported by the Dash documentation. Therefore, while it may help solve your issue, it is not a reliable or recommended solution.

We recommend that you update your version of Dash and other related components to the latest available version. Please make sure that you have the latest versions of the dash, dash-core-components, dash-html-components, dash-table and dash-design-kit libraries installed. This ensures that you are using the most compatible version of React for your version of Dash.

Also, consider creating a React app as suggested by a colleague above.

nickmelnikov82 avatar Jun 01 '23 11:06 nickmelnikov82

Resolving this will be a blocker for rolling out React 18 as the default version for Dash.

ndrezn avatar Mar 21 '24 15:03 ndrezn

Noting this blocks: https://github.com/plotly/dash/issues/2254. @emilykl , tagging you to take a look in this one when you're back from eclipse watching! Would be great to formally bump to React 18 with Dash 2.17.

ndrezn avatar Apr 08 '24 15:04 ndrezn

Took an initial look -- I think the weird behavior is likely related to the fact that we are calling setState() inside the render() method of ddk.Notification (see here -- note that the DDK repo is private). This raises a warning even in previous versions of React but plausibly the rendering changes in React 18 have made it so the pattern used in ddk.Notification no longer works.

Happy to dig deeper but this is beyond my area of expertise -- wondering @HammadTheOne whether you have thoughts or would be willing to take a look?

cc @ndrezn

emilykl avatar Apr 15 '24 22:04 emilykl

@emilykl leaving with you to follow up in a 1:1 with Hammad next week and see what changes are required.

ndrezn avatar Apr 21 '24 17:04 ndrezn