LouisBarranqueiro / reapop

:postbox: A simple and customizable React notifications system
https://louisbarranqueiro.github.io/reapop
MIT License
1.55k stars 78 forks source link

`ReactNode` vs `string` for message #525

Closed polewskm closed 7 months ago

polewskm commented 1 year ago

Description

Would it be possible for the message property to be of type ReactNode vs string?

Explanation / motivation

This would allow any React content to be displayed in the notification vs strings or strings with embedded HTML.

Additional information

LouisBarranqueiro commented 1 year ago

👋 @polewskm , sorry for the very late reply. Could you please share a concrete example of what you are trying to do? Why string or HTML string do not cover your needs?

Would it be possible for the message property to be of type ReactNode vs string?

It is not supported by default but you can adapt the library to create a notification (Notification) with a given a React component (e.g: Notification.messageComponent), and then you would create your own React Notification to display such notifications.

polewskm commented 1 year ago

Current use-case with message as string

import React from 'react'
import {useNotifications} from 'reapop'

const AComponent = () => {
    // 1. Retrieve the action to create/update a notification.
    const {notify} = useNotifications()

    useEffect(() => {
        // 2. Create a notification.
        notify('Welcome to the documentation', 'info')
    }, [])

    return (
        ...
    )
}

Proposed use-case with message as ReactNode

import React from 'react'
import {useNotifications} from 'reapop'

const AComponent = () => {
    // 1. Retrieve the action to create/update a notification.
    const {notify} = useNotifications()

    useEffect(() => {
        // 2. Create a notification.
        notify(<div class='root'><p>Welcome to the documentation</p><p>other content...</p></div>, 'info')
    }, [])

    return (
        ...
    )
}

The purpose of this is to use native react rendering for HTML content vs unsafe rendering of HTML raw strings via the dangerouslySetInnerHTML property that you are currently doing.

LouisBarranqueiro commented 1 year ago

The purpose of this is to use native react rendering for HTML content vs unsafe rendering of HTML raw strings via the dangerouslySetInnerHTML property that you are currently doing.

<div class='root'><p>Welcome to the documentation</p><p>other content...</p></div> is JSX and converted to a React element by your JS compiler like babel. React Elements are not serializable, and therefore can't be put stored in Redux. While we could still hack something to make it work for simple components, an HTML string will do the work perfectly. There is nothing wrong with dangerouslySetInnerHTML attribute as long as you know how to use it. That's why the attribute exists. However, it is marked as dangerous to remind developers that it could be in certain situations and make sure they don't pass any data to it.

The purpose of this library is to propose a simple, yet powerful API but most importantly stable to add notifications to your application. A message as a string fulfills all the use cases this library is built for.

I would suggest building your own custom Notification components and add extra properties to notifications to control/display customized information instead of attempting to store non-serializable (React elements) data in the state of React/Redux.

What these custom notifications would display? In the example you shared, you can replace it with an HTML string but I guess you want to build something more complex with logic into it, right?

LouisBarranqueiro commented 7 months ago

@polewskm I'm closing this inactive issue. Please re-open it if needed.