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

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

Closed
rymndhng opened this issue Apr 28, 2023 · 6 comments
Assignees

Comments

@rymndhng
Copy link

rymndhng commented Apr 28, 2023

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

@Bidek56
Copy link

Bidek56 commented May 16, 2023

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. :)

@nickmelnikov82
Copy link
Contributor

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.

@ndrezn
Copy link
Member

ndrezn commented Mar 21, 2024

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

@ndrezn
Copy link
Member

ndrezn commented Apr 8, 2024

Noting this blocks: #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.

@emilykl
Copy link
Contributor

emilykl commented Apr 15, 2024

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

@ndrezn
Copy link
Member

ndrezn commented Apr 21, 2024

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

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

No branches or pull requests

5 participants