Material-ui: Maybe a fix for unknown props in [RefreshIndicator]...

Created on 30 Sep 2016 · 6Comments · Source: mui-org/material-ui

Problem description

I kept receiving Warning: Unknown props left, top, status, percentage on

tag. Remove these props from the element. For details, see https://fb.me/react-unknown-prop
in div (created by Paper)
in Paper (created by RefreshIndicator).

I did some testing and logging within the npm_modules/material-ui/RefreshIndicator code and seemed to isolate the problem within the render block where props are being merged and assigned to the parent Paper element. As a test, I excluded the 'other' props from the merge and the warnings have disappeared without any apparent negative consequences in the components intended behavior.

Steps to reproduce

This would occur when loading a route that has a RefreshIndicator fire up in response to props being set that toggle the status (thus loading true/false).

Versions

Material-UI: 15.3.2
React: 0.16.0-rc2
Browser: chrome

Description

So, I went into the RefreshIndicator source, and at line 356, simply commented out where the 'other' props are being merged and passed to the Paper element being created and the errors have gone away. I have yet to notice any side effects or bad behavior. The warnings are gone and the components still perform as expected.

See below in the render block where I did a test to comment out the merging of 'other' props

key: 'render',
    value: function render() {
      var _props = this.props;
      var style = _props.style;
      var other = (0, _objectWithoutProperties3.default)(_props, ['style']);

      var styles = getStyles(this.props, this.context);

      return _react2.default.createElement(
        _Paper2.default,
        (0, _extends3.default)({
          circle: true,
          style: (0, _simpleAssign2.default)(styles.root, style)
        }),//, other), <!---- here!!
        this.renderChildren()
      );
    }

Anyways, not sure if this is the proper place for this, but thought I'd pass along.

Images & references

-->

Source

verenea

👍6

Most helpful comment

I'm sorry guys for the regression. I will be released in the 0.16.1 version.

oliviertassinari on 9 Oct 2016

👍4

All 6 comments

I see this as well on a Mocha test. I had to comment out part of babel-template code that was suppressing Mocha's ability to log the test failure.

> NODE_PATH=./app mocha --opts mocha.opts



  <Header />
    ✓ renders
    ✓ has one <div>
    ✓ <div> uses the correct classes
    ✓ has one <h1>

  <App />
Warning: Unknown prop `onTouchTap` on <button> tag. Remove this prop from the element. For details, see https://fb.me/react-unknown-prop
    in button (created by EnhancedButton)
    in EnhancedButton (created by IconButton)
    in IconButton (created by AppBar)
    in div (created by Paper)
    in Paper (created by AppBar)
    in AppBar (created by App)
    in div (created by App)
    in MuiThemeProvider (created by App)
    in App (created by withRouter(App))
    in withRouter(App) (created by Constructor)
    in Constructor
    ✓ renders
    ✓ has a <MuiThemeProvider />
    ✓ renders children when passed in

  <About />
    ✓ renders

  <Home />
    ✓ renders
    ✓ has a title header
    ✓ has links to other routes


  11 passing (427ms)

rpoitras on 30 Sep 2016

material-ui: 0.15.4
react: 15.3.2

rpoitras on 30 Sep 2016

http://www.material-ui.com/#/components/refresh-indicator

Warning: Unknown props `left`, `top`, `loadingColor`, `status`, `percentage` on <div> tag. Remove these props from the element. For details, see https://fb.me/react-unknown-prop

    "react": "^15.3.2",
    "material-ui": "^0.16.0",

avevlad on 5 Oct 2016

I've also got this issue. The actual (pre-Babel) source looks like this:

render() {
    const {
      style,
      ...other,
    } = this.props;

    const styles = getStyles(this.props, this.context);

    return (
      <Paper
        circle={true}
        style={Object.assign(styles.root, style)}
        {...other}
      >
        {this.renderChildren()}
      </Paper>
    );
  }

Is there anything that <Paper /> would ever need passed down to it from the <RefreshIndicator /> call site? If not, the simplest fix would be simply to remove {...other}. If there is, then they should probably be properly defined in the props list.

According to #5334 this change was introduced in PR #5054 which suspiciously has the Has tests checkbox unmarked. Not sure why it was accepted without tests, but there's the root cause.