timolins / react-hot-toast

Smoking Hot React Notifications 🔥
https://react-hot-toast.com
MIT License
9.84k stars 331 forks source link

Warning validateDOMNesting in case toast message contains <div> #5

Closed ambroseus closed 3 years ago

ambroseus commented 3 years ago

Got warning in Chrome: validateDOMNesting(...): <div> cannot appear as a descendant of <p>. with material-ui's <Alert> inside toast message (or any other div)

Possible solution: replace 'p' with 'div' https://github.com/timolins/react-hot-toast/blob/9045f1ca73f7ec5301dc5a7bbea4a19f106384c0/src/components/toast-bar.tsx#L39

timolins commented 3 years ago

Thanks, good catch. Will fix this with the next release.

albannurkollari commented 3 years ago

Replacement is never a good choice! What if the developer has specific styling to just that <p> element?

I'd rather have the library throw an error (or at least log as error and not render the toaster at all) than modify the input from the dev.

Remember, if you silently fix a mistake from a developer you inadvertly are not helping them and thus they are stuck with that knowledge until someone points it out for them.

Best choice would be if you provide an option for silently logging errors instead of throwing it in such scenarios, if the user wants to pass it as an argument.

ambroseus commented 3 years ago

yep, also in case of default styled <p> it's not possible to easy style custom toast such as <Alert> from material-ui, for ex. Selection_009

ambroseus commented 3 years ago

It will be good to have the ability to override default toast container (<Message> component)

timolins commented 3 years ago

@albannurkollari Not sure if I got that correctly, but I think by replacing @ambroseus was referring to using div by default instead of p. This shouldn't have too many drawbacks other than being a bit less semantic. The error itself wasn't thrown by react-hot-toast, but by React itself because it renders incorrect HTML (nesting a div inside a p tag).

That said I agree that it should be possible to render a custom component, without having to go headless. Not sure yet what's the best API here since there are two components (ToastBar + Message) that need to be replaced. Maybe a disableStyles option would do the trick as well.

ambroseus commented 3 years ago

@timolins maybe something like this (override Message component at all):


  <Toaster toastOptions={{
      MessageContainer: (props) => <div {...props} />
  }} />
albannurkollari commented 3 years ago

@timolins Thanks for clarification, I wasn't aware that fhe context was for the default wrapper of the Toaster! And ofc, the error/warning comes from React.

timolins commented 3 years ago

I'll open a new issue regarding the message customisation.