schiehll / react-alert

alerts for React
MIT License
607 stars 99 forks source link

Use React context for accessing message service #50

Closed williamboman closed 6 years ago

williamboman commented 7 years ago

What do you think about moving to a context-based AlertContainer with injection of the message service via a higher-order component?

High-level view of what I'm suggesting (very similar to how react-intl works):

import {AlertProvider, injectAlert} from 'react-alert'

export class App extends Component {

    render() {
        return (
            <AlertProvider>
                <Sidebar />
                <Content>
                    <Home />
                </Content>
            </AlertProvider>
        )
    }
}

@injectAlert
class Home extends Component {

    _alert = () => {
        // this.props.alert is provided by injectAlert higher-order-component which knows which AlertProvider
        // to work against via React context
        this.props.alert.show(<DummyAlert />)
    }

    render() {
        <div className='Home'>
            <button onClick={this._alert}>Alert!</button>
        </div>
    }
}

const DummyAlert = () => (
    <h1>Hello world</h1>
)

Pros of this is no need to work against both the React ref API as well as react-alert's component class itself (both being a bit of an anti-pattern in React-world). It's also much easier to consume this library by being able to inject the alert service where it's needed, without having to pass it as a prop through the entire component tree.

schiehll commented 6 years ago

Yep, that's kinda how v3 works, haven't notice this issue before as I wasn't having much time, but I'm glad someone agrees that this is a good idea haha