fkhadra / react-toastify

React notification made easy 🚀 !
https://fkhadra.github.io/react-toastify/introduction
MIT License
12.72k stars 700 forks source link

ToastContainer lazy behavior #608

Closed rivajunior closed 2 years ago

rivajunior commented 3 years ago

Do you want to request a feature or report a bug? Is a feature request, but I think it is also a bug

What is the current behavior?

When an app uses the configure method

// app/src/main.js supossed app starter file
toast.configure(options)

Only a root DOM element to render the toast is created on demand, when toast function or its variations (eg. toast.info()) is called. But is really what should be considered as "lazy"?

// react-toastify/src/core/toast.tsx
if (lazy && canUseDom) {
      lazy = false;
      containerDomNode = document.createElement('div');
      document.body.appendChild(containerDomNode);
      render(<ToastContainer {...containerConfig} />, containerDomNode);
}

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your CodeSandbox (https://codesandbox.io/s/new) example below:

What is the expected behavior?

I expect the use of React.lazy function to really load ToastContainer lazy.

import { Suspense } from 'react'

if (lazy && canUseDom) {
  const ToastContainer = lazy(() => import('../components/ToastContainer'));

  lazy = false;
  containerDomNode = document.createElement('div');
  document.body.appendChild(containerDomNode);
  render(
    <Suspense fallback={<div />}>
      <ToastContainer {...containerConfig} />
    </Suspense>,
     containerDomNode
 );
}

I don't know if it really a benefit for the use. But has the current behavior any benefit besides one DOM node less at the application start?

Anyway. That was what I thought about "lazy" on the documentation.

fkhadra commented 2 years ago

I'll update the doc to make it clear. And maybe update the implementation when react 18 ship. Closing for now