Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stateless function components cannot be given refs. Attempts to access this ref will fail #861

Closed
nabinked opened this issue Jul 6, 2018 · 17 comments
Assignees

Comments

@nabinked
Copy link

nabinked commented Jul 6, 2018

Following errors thrown while trying to render a Line Chart from the demo here.

Image

Steps to reproduce

  • Create a new react app using CRA
  • Add react-vis to dependencies
  • Create a Component with following content
import React from 'react';

import {
  XYPlot,
  XAxis,
  YAxis,
  HorizontalGridLines,
  VerticalGridLines,
  LineSeries
} from 'react-vis';

export default class Example extends React.Component {
  render() {
    return (
      <XYPlot
        width={300}
        height={300}>
        <HorizontalGridLines />
        <VerticalGridLines />
        <XAxis title="X Axis" position="start"/>
        <YAxis title="Y Axis"/>
        <LineSeries
          className="first-series"
          data={[
            {x: 1, y: 3},
            {x: 2, y: 5},
            {x: 3, y: 15},
            {x: 4, y: 12}
          ]}/>
        <LineSeries
          className="second-series"
          data={null}/>
        <LineSeries
          className="third-series"
          curve={'curveMonotoneX'}
          style={{
            strokeDasharray: '2 2'
          }}
          data={[
            {x: 1, y: 10},
            {x: 2, y: 4},
            {x: 3, y: 2},
            {x: 4, y: 15}
          ]}
          strokeDasharray="7, 3"
          />
        <LineSeries
          className="fourth-series"
          data={[
            {x: 1, y: 7},
            {x: 2, y: 11},
            {x: 3, y: 9},
            {x: 4, y: 2}
          ]}/>
      </XYPlot>
    );
  }
}
  • Import it to App.js
import React, { Component } from 'react';
import LineChart from './LineChart';
import './App.css';

class App extends Component {
  render() {
    return (
      <LineChart />
    );
  }
}

export default App;
  • Do a yarn start and see the console.

  • package.json

{
  "name": "my-app",
  "version": "0.1.0",
  "private": true,
  "dependencies": {
    "react": "^16.4.1",
    "react-dom": "^16.4.1",
    "react-scripts": "1.1.4",
    "react-vis": "^1.10.1"
  },
  "scripts": {
    "start": "react-scripts start",
    "build": "react-scripts build",
    "test": "react-scripts test --env=jsdom",
    "eject": "react-scripts eject"
  }
}

My guess is that the XYPlot component in function _getClonedChildComponents is adding ref to the props of the child components, while the children can easily be SFCs (In the example above the components HorizontalGridLines,VerticalGridLines,XAxis,YAxis are all SFC inside XYPlot component, hence the 4 exceptions) and in that case the exception is thrown.

@chrisjones0517
Copy link

I'm having exactly the same issue on a budget/financial planning application I'm working on.

@ghost
Copy link

ghost commented Jul 9, 2018

Same here. Version of "react-vis": "^1.10.1"
The code os above:

import React from 'react'
import { XYPlot, FlexibleWidthXYPlot, makeWidthFlexible, XAxis, YAxis, HorizontalGridLines, LineMarkSeries, CustomSVGSeries } from 'react-vis'

export default class Chart extends React.Component {
    className = 'Chart'
    blueColor = "#0078ff"
    lineColor = this.blueColor
    markStyle = { stroke: this.blueColor, fill: this.blueColor }
    render() {
        const { width, height } = this.props;
        let plot = (
            <FlexibleWidthXYPlot
                height={height}>
                <HorizontalGridLines />
                <LineMarkSeries
                    color={this.blueColor}
                    data={[
                        { x: 1, y: 100 },
                        { x: 2, y: 400 },
                        { x: 3, y: 300 },
                        { x: 4, y: 500 },
                        { x: 5, y: 600 },
                        { x: 6, y: 400 },
                        { x: 7, y: 500 },
                    ]} />
                <CustomSVGSeries
                    // className="custom-marking"
                    customComponent="square"
                    data={[
                        { x: 1, y: 100, size: 6, style: { stroke: this.blueColor, fill: this.blueColor } },
                        { x: 2, y: 400, size: 6, style: { stroke: this.blueColor, fill: this.blueColor } },
                        { x: 3, y: 300, size: 6, style: { stroke: this.blueColor, fill: this.blueColor } },
                        { x: 4, y: 500, size: 6, style: { stroke: this.blueColor, fill: this.blueColor } },
                        { x: 5, y: 600, size: 6, style: { stroke: this.blueColor, fill: this.blueColor } },
                        { x: 6, y: 400, size: 6, style: { stroke: this.blueColor, fill: this.blueColor } },
                        { x: 7, y: 500, size: 6, style: { stroke: this.blueColor, fill: this.blueColor } },
                    ]} />
                {/* <XAxis /> */}
                <YAxis />
            </FlexibleWidthXYPlot>
        );
        return plot;
        // return makeWidthFlexible(plot);
    }
}

@jdominiczak
Copy link

Same for me. Reverting to 1.10.0 removes the errors. 00ffe61 appears to be the cause.

@mcnuttandrew
Copy link
Contributor

@benshope mind taking a look at this?

@benshope benshope self-assigned this Jul 10, 2018
@benshope
Copy link
Contributor

@mcnuttandrew on it 🐛 🔨

@benshope
Copy link
Contributor

Should be fixed in 1.10.2

@ericzon
Copy link

ericzon commented Jul 11, 2018

I've just tried in 1.10.2 and still receiving the same error with the example code.

@jdominiczak
Copy link

Same for me @ericzon.

@benshope
Copy link
Contributor

Interesting, okay, re-opening

@benshope
Copy link
Contributor

Should 🙏 be fixed in 1.10.3

@jdominiczak
Copy link

Looks like that fixed it. Thanks @benshope and @mcnuttandrew for the work on this.

@markov00
Copy link
Contributor

@benshope @mcnuttandrew I don't think this solves correctly the issue and I've found few problem on that:

  1. If I've understood correctly from the react internals, the child object you are testing for it's stateless nature is a ReactElement object and not a Component object. Debugging your code you can see that the child is a ReactElement with few properties as stated in the link provided. If you want add refs only to non stateful components you have to check the type property on the child like:
 ...(dataProps && child.type.prototype child.type.prototype.render) ? { refs } : {}

In the current implementation you will never add any ref because you are checking for child.prototype that doesn't exist on a child and the conditions are wrong (we need to add refs only if we have dataProps and the child is a statefull component, so with prototype and render methods). (see also your series-utils isSeriesChild function that checks for series and use type https://github.com/uber/react-vis/blob/master/src/utils/series-utils.js#L31 )

  1. I think there was some problem with the npm publishing, because if I use the version 1.10.2 this commit is not included: 1ab5cc7
    The version 1.10.2 seems the publication of the version 1.10.1 or something in between.

markov00 added a commit to elastic/eui that referenced this issue Jul 13, 2018
* Updated react-vis to 1.10.2

The features we introduced in XYPlot are now merged into react-vis after uber/react-vis#857. The XYPlotExtended class is removed.

The div aroung the XYPlot is also removed since it's not required. All snapshots are updated after this removal.

* Updated CHANGELOG

* Reverting 492d28a to keep the wrapping div around the chart

* Fixed the react-vis version to the stable one.

Check this issue comments to see the status: uber/react-vis#861
@geyang
Copy link

geyang commented Jul 16, 2018

@benshope after upgrading to 1.10.3, it looks like the zoomable plot doesn't work anymore. In fact inspection shows that all mouse events are not triggering.

I tried to run the examples directly in the repo from source they work well. But when used with the compiled 1.10.3 it doesn't work anymore.

@mcnuttandrew
Copy link
Contributor

@episodeyang i think there's one more patch coming down the pipe see #874

@mcnuttandrew mcnuttandrew reopened this Jul 16, 2018
@geyang
Copy link

geyang commented Jul 16, 2018

@mcnuttandrew Thanks Andrew! down grading to 1.10.1 fixed the handler responses. Looking forward to #874.

btw congrats on UChicago :P didn't know you started last year.

@benshope
Copy link
Contributor

Should be fixed in 1.10.4 by #874

@fabienjuif
Copy link

I updated to 1.10.4 and it's all OK here 👍

Thank you @benshope and thank you @markov00 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants