Add OverlayToaster.createAsync method to support React 18

[gluxon]

PR Stack

• Ensure OverlayToaster tests are repeatable #6601
• Add OverlayToaster.createAsync method to support React 18 #6599

Checklist

• Includes tests
• Update documentation

Changes

Fixes #6239. A new OverlayToaster.createAsync() utility is added alongside OverlayToaster.create().

This asynchronous function allows a custom domRenderer utility to be passed to better support codebases on React 18.

import { OverlayToaster } from "@blueprintjs/core";
import { createRoot } from "react-dom/client";

const toaster = await OverlayToaster.createAsync(/* props */ {}, {
  domRenderer: (toaster, containerElement) => createRoot(containerElement).render(toaster),
});

toaster.show({ message: "Hello React 18!" })

The existing OverlayToaster.create() method uses ReactDOM.render without any ability to customize this call, which results in a console warning when upgrading to React 18.

Why is the new API asynchronous?

The change to Blueprint's API reflects a change in React's API. The new createRoot function from react-dom/client no longer renders components synchronously.

import * as React from "react";
import { createRoot } from "react-dom/client";

const toaster = React.createRef<OverlayToaster>();

createRoot(containerElement)
  .render(<OverlayToaster {...props} ref={toaster} usePortal={false} />)
  
// OverlayToaster render function is not yet executed.

// ‼️ This is null ‼️
ref.current

setTimeout(() => {
  // Okay, now the OverlayToaster render() function has ran.
  ref.current // now available
}, 0)

This is different than ReactDOM.render, which does synchronously populate the ref.

import * as React from "react";
import * as ReactDOM from "react-dom";

const toaster = React.createRef<OverlayToaster>();

ReactDOM.render(
  <OverlayToaster {...props} ref={toaster} usePortal={false} />,
  containerElement
);

// OverlayToaster render function has ran by this point.

ref.current // instance of <OverlayToaster />

This seems to be an intentional change in React 18. See the “What about the render callback?” section under Replacing render with createRoot.

Why do I need to pass in a custom domRenderer value for React 18?

The createRoot function is an import on react-dom/client. Blueprint is an NPM library that needs to support multiple bundlers (e.g. Webpack, Vite). Most bundlers will fail if it encounters a non-existent submodule import. Since the current major version of Blueprint needs to support React 16 to 18, imports into 18-specific code paths can't be made directly in Blueprint.

As a workaround, consumers of Blueprint can provide the createRoot function to Blueprint. This is an application of dependency injection.

When Blueprint drops support for React 16 in a future major version, the domRenderer option will change its current default from ReactDOM.render to a function using the new createRoot API. This breaking change will make OverlayToaster.createAsync easier to use in the future.

How do I migrate?

The most one-to-one conversion would be from:

OverlayToaster.create().show({ message: "Hello!" });

to:

OverlayToaster.createAsync().then(toaster => toaster.show({ message: "Hello!" }));

You may notice:

1. We're swallowing errors that happen when rendering the component. There's no rejection handler on the promise object.
2. The OverlayToaster component is never unmounted or removed from the DOM after the message is dismissed.

Both of these are true of the original synchronous OverlayToaster.create() API. The new API makes these existing problems more apparent. We recommend mounting the toaster directly to your React application tree if the first problem is a concern, and sharing the Toaster to avoid the second problem.

Reviewers should focus on:

• Whether the new API makes sense.
• Reviewing each commit individually should be easier than reviewing all changes at once. Hiding whitespace only changes should also make the diff easier to look at since a few tests were nested.
• Several refactors had to be made to the OverlayToaster tests to make them reusable for both OverlayToaster.create and OverlayToaster.createAsync.
• I briefly spent some time attempting to add a React 18 dependency for testing, but hit issues with NPM aliases discovering @types/react-dom@18. I can take another stab if we think a React 18 test is valuable to commit, but it's a bit hard to have multiple version of React in the same codebase, even for testing.

[gluxon]

We were previously calling ReactDOM.unmountComponentAtNode on the wrong DOM element. The OverlayToaster.create() method creates a new <div> within testsContainerElement to render on.

[gluxon]

Without the additional cleanup in a few tests that call OverlayToaster.create() directly (instead of using the existing Toaster instance), I was seeing the "does not attach toast container to body on script load" test fail.

[gluxon]

I would actually swap the recommended approach here since OverlayToaster.create() and OverlayToaster.createAsync() both leak memory and add a DOM node to the page forever. Thoughts?

[adidahiya]

Ok, sure, I'm down with that, we can change the recommended approach. So we'd reverse the order of approaches: 3 becomes 1 (recommended), and 1 becomes 3. Since it seems like most people prefer having an imperative API in their application to trigger toasts rather than storing a list of Toasts in app state and rendering <Toast> themselves.

It's worth adding more docs under the "Static usage" section around L105 with your note about memory leaks and leaving a DOM node on the page forever. Also perhaps update the code snippets there to use createAsync instead of create?

[gluxon]

Ok, sure, I'm down with that, we can change the recommended approach. So we'd reverse the order of approaches: 3 becomes 1 (recommended), and 1 becomes 3. Since it seems like most people prefer having an imperative API in their application to trigger toasts rather than storing a list of Toasts in app state and rendering <Toast> themselves.

I pushed up a few commits that switched 1 and 3, but decided to walk this back. The primary factor that changed my mind was that it becomes harder to pass the OverlayToaster ref down the React component tree. I think that will be a bigger point of confusion than I originally thought when I suggested switching the recommendations. Thanks for being open to my suggestion.

The latest push keeps the current recommendation since it may be the best of all options, but adds more notes around only creating one Toaster instance.

It's worth adding more docs under the "Static usage" section around L105 with your note about memory leaks and leaving a DOM node on the page forever.

Can do! Added in d375696.

Also perhaps update the code snippets there to use createAsync instead of create?

Good suggestion. Pushed up a commit that switches the docs and code snippets to createAsync here: 9b1fb93

[adidahiya]

Add OverlayToaster.createAsync test

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

[adidahiya]

Add OverlayToaster.createAsync test

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

[adidahiya]

Add OverlayToaster.createAsync test

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

[adidahiya]

thanks for improving the test suite, nice to see the amount of code re-use there 👍🏽

code changes & behavior look good, I'm requesting some docs changes in this review

would you mind adding a new (small) docs example using OverlayToaster.createAsync() which gets shown in this part of the docs? it doesn't need to have options:

this will help us to have a quick & easy way to test the imperative API interactively in the docs preview. Currently, toastExample.tsx only uses the declarative API.

[adidahiya]

Ok, sure, I'm down with that, we can change the recommended approach. So we'd reverse the order of approaches: 3 becomes 1 (recommended), and 1 becomes 3. Since it seems like most people prefer having an imperative API in their application to trigger toasts rather than storing a list of Toasts in app state and rendering <Toast> themselves.

It's worth adding more docs under the "Static usage" section around L105 with your note about memory leaks and leaving a DOM node on the page forever. Also perhaps update the code snippets there to use createAsync instead of create?

[adidahiya]

nit: which documentation site? Blueprint or React? Might be useful to just add a link

[gluxon]

Agree with it being useful to just add a link. This message was copied from the TOASTER_CREATE_NULL. I'll update the original message as well. 4779ba2

[gluxon]

Thanks for all the feedback! Agree that the docs need to have some improvements. I'm still working through a few more clarifications and will move the PR to draft mode until I wrap those up.

[adidahiya]

Move "React component usage example" further up

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

[gluxon]

would you mind adding a new (small) docs example using OverlayToaster.createAsync() which gets shown in this part of the docs? it doesn't need to have options

Can do! Done in commit 9bdf637.

I ended up moving this to the Example section since the code snippets were very similar to what the interactive example implements. If that's not a good place, I can move it back to where you suggested.

[gluxon]

Agree with it being useful to just add a link. This message was copied from the TOASTER_CREATE_NULL. I'll update the original message as well. 4779ba2

[gluxon]

Ok, sure, I'm down with that, we can change the recommended approach. So we'd reverse the order of approaches: 3 becomes 1 (recommended), and 1 becomes 3. Since it seems like most people prefer having an imperative API in their application to trigger toasts rather than storing a list of Toasts in app state and rendering <Toast> themselves.

I pushed up a few commits that switched 1 and 3, but decided to walk this back. The primary factor that changed my mind was that it becomes harder to pass the OverlayToaster ref down the React component tree. I think that will be a bigger point of confusion than I originally thought when I suggested switching the recommendations. Thanks for being open to my suggestion.

The latest push keeps the current recommendation since it may be the best of all options, but adds more notes around only creating one Toaster instance.

It's worth adding more docs under the "Static usage" section around L105 with your note about memory leaks and leaving a DOM node on the page forever.

Can do! Added in d375696.

Also perhaps update the code snippets there to use createAsync instead of create?

Good suggestion. Pushed up a commit that switches the docs and code snippets to createAsync here: 9b1fb93

[gluxon]

This should be ready for another review after the holidays. I wanted to finish my in-progress changes before I lost context over the next few weeks. Please feel free to ignore this until next year. Thanks!

[adidahiya]

Add React 18 domRenderer example

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

[adidahiya]

new example works well in docs preview

[adidahiya]

these docs look good 👍🏽

[gluxon]

Thanks!