component does not work with React 16

[abramobagnara]

The cause is the use of ReactDOM.render return value, that is an obsolete feature (also in React 15).
We should use ref instead.

https://facebook.github.io/react/docs/react-dom.html#render

[paranoidjk]

https://github.com/react-component/notification/blob/master/src/Notification.jsx#L99

[paranoidjk]

I think it should works fine, see the react document:

Render a React element into the DOM in the supplied container and return a reference to the component (or returns null for stateless components).

[paranoidjk]

Please give a reproducible demo, and the detail error info.

[abramobagnara]

You can reproduce the crash using npm install --save react@next react-dom@next
and putting this reduced testcase in examples/test.js:

/* eslint-disable no-console */
import 'rc-notification/assets/index.less';
import Notification from 'rc-notification';
import React from 'react';
import ReactDOM from 'react-dom';
let notification;

function simpleFn() {
  notification.notice({
    content: <span>simple show</span>,
    onClose() {
      console.log('simple close');
    },
  });
}

class App extends React.Component {
  render() {
    return <div>hello</div>
  }
  componentDidMount() {
    notification = Notification.newInstance({});
    simpleFn();
  }
}

ReactDOM.render(<App/>, document.getElementById('__react-content'));

You will see in console:

TypeError: Cannot read property 'add' of null

[abramobagnara]

See facebook/react#10309 for the clarification why this is a breaking changes in React 16 that will remain.

The point is that to use the return value of ReactDOM.render should be avoided in general.

[benjycui]

How about an online demo?

Please provide a re-producible demo: http://codepen.io/benjycui/pen/aJooRE?editors=0011

Edit: @paranoidjk , it seems that rc-notification hasn't provided dist file? See: https://unpkg.com/rc-notification@2.0.2/

[benjycui]

ping~ @paranoidjk

[paranoidjk]

@benjycui react-component/react-component.github.io#14

It's caused by rc-tools break change.

[paranoidjk]

@abramobagnara Please provide a reproducible demo, eg: https://codepen.io/paranoidjk/pen/NvxmKQ?editors=0011

@benjycui I found our umd dist do not handle add-module-exports, users must use

const Notification = window['rc-notification'].default;

[abramobagnara]

This is a reproducible demo:

https://codepen.io/abramo/pen/ZJQgBL?editors=1111

[bkniffler]

Any news on this one?

[henrycity]

I have the same error TypeError: Cannot read property 'add' of null as well. After upgrading to React 16, it doesn't work anymore. This also affects Toast component in antd-mobile.

[tylerlong]

I confirm that it doesn't work with React 16

[benjycui]

After upgrade to React@16...

If we use Notification.newInstance in lifecycle of a React component, we cannot get a ref from ReactDOM.render https://github.com/react-component/notification/blob/master/src/Notification.jsx#L99

But if we don't use Notification.newInstance in lifecycle, it works fine, just like http://react-component.github.io/notification/examples/simple.html

I think we must fix this issue, for developers may use antd.notification in lifecycle. And the simplest way may be make Notification.newInstance return a promise, instead of a ref directly.

cc @afc163 @paranoidjk

[paranoidjk]

facebook/react#10294

ReactDOM.render() and ReactDOM.unstable_renderIntoContainer() now return null if called from inside a lifecycle method.
To work around this, you can either use the new portal API or refs.

Looks like we need rewrite this with new portal api.

But i am pretty busy on another project these days. @silentcloud @benjycui @yesmeck @zhang740 Anybody want to handle this?

[yesmeck]

react-component/trigger#69

[benjycui]

Looks like we need rewrite this with new portal api.

Actually, we don't need to..

Even if we rewrite with React.createPortal, we still need to make Notification.newInstance async.

[benjycui]

See: #27

We cannot use React.createPortal, for we don't want to change API in antd.