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

Cannot update a component (ConnectFunction) while rendering a different component (SpecsParserComponent). To locate the bad setState() call inside `SpecsParserComponent #720

Closed
2 tasks
flavio-kadata opened this issue Jun 24, 2020 · 14 comments · Fixed by #723
Labels
bug Something isn't working released Issue released publicly

Comments

@flavio-kadata
Copy link

Describe the bug

I'm trying to plot a very simple Graph in my app, but I'm curently facing this weird issue:

Warning: Cannot update a component (`ConnectFunction`) while rendering a different component (`SpecsParserComponent`). To locate the bad setState() call inside `SpecsParserComponent`, follow the stack trace as described in https://fb.me/setstate-in-render

To Reproduce

I'm currently using

  • react 16.3.1
  • @elastic/charts 19.5.2

I have a parent component which calls a child component where I have something like this:

import React from 'react';

import {
    Chart,
    Settings,
    Axis,
    LineSeries,
    niceTimeFormatByDay,
    timeFormatter
} from '@elastic/charts';

export interface SimulationStatsGraphSeries {
    id: string;
    name: string;
    data: Array<[Date, number]>;
}

export interface SimulationStatsGraphProps {
    simulationStatsGraphSeries: { [serie: string]: SimulationStatsGraphSeries };
}

const SimulationStatsGraph: React.FC<SimulationStatsGraphProps> = (
    props: SimulationStatsGraphProps
) => {
    const { simulationStatsGraphSeries } = props;

    return (
        <Chart size={{ height: 200 }}>
            <Settings showLegend={true} legendPosition="bottom" />

            <Axis
                title={new Date().toISOString()}
                id="bottom-axis"
                position="bottom"
                tickFormat={timeFormatter(niceTimeFormatByDay(1))}
                showGridLines
            />
            <Axis id="left-axis" position="left" showGridLines />

            {Object.keys(simulationStatsGraphSeries).map((k: string) => (
                <LineSeries
                    key={k}
                    id={simulationStatsGraphSeries[k].id}
                    name={simulationStatsGraphSeries[k].name}
                    data={simulationStatsGraphSeries[k].data}
                    xScaleType="time"
                    xAccessor={0}
                    yAccessors={[1]}
                />
            ))}
        </Chart>
    );
};

export default SimulationStatsGraph;

Expected behaviour

Graph plotted.

Screenshots

Bildschirmfoto vom 2020-06-24 18-56-58

Version (please complete the following information):

  • OS: Linux
  • Browser: Chrome
  • Elastic Charts: 19.5.2

Additional context
Add any other context about the problem here.

Errors in browser console

index.js:1 Warning: Cannot update a component (`ConnectFunction`) while rendering a different component (`SpecsParserComponent`). To locate the bad setState() call inside `SpecsParserComponent`, follow the stack trace as described in https://fb.me/setstate-in-render
    in SpecsParserComponent (created by ConnectFunction)
    in ConnectFunction (created by Chart)
    in div (created by Chart)
    in Provider (created by Chart)
    in Chart (at SimulationStatsGraph.tsx:28)
    in SimulationStatsGraph (at SimulationStats.tsx:503)
    in div (created by EuiFlexItem)
    in EuiFlexItem (at SimulationStats.tsx:502)
    in div (created by EuiFlexGroup)
    in EuiFlexGroup (at SimulationStats.tsx:476)
    in div (created by EuiFlexItem)
    in EuiFlexItem (at SimulationStats.tsx:475)
    in div (created by EuiFlexGroup)
    in EuiFlexGroup (at SimulationStats.tsx:451)
    in div (at SimulationStats.tsx:450)
    in SimulationStats (at SimulationResults.tsx:83)
    in div (created by EuiFlexItem)
    in EuiFlexItem (at SimulationResults.tsx:82)
    in div (created by EuiFlexGroup)
    in EuiFlexGroup (at SimulationResults.tsx:81)
    in SimulationResults (at Simulation.tsx:86)
    in div (created by EuiFlexItem)
    in EuiFlexItem (at Simulation.tsx:85)
    in div (created by EuiFlexGroup)
    in EuiFlexGroup (at Simulation.tsx:81)
    in div (created by EuiPageContentBody)
    in EuiPageContentBody (at Page.tsx:55)
    in div (created by EuiPanel)
    in EuiPanel (created by EuiPageContent)
    in EuiPageContent (at Page.tsx:44)
    in div (created by EuiPageBody)
    in EuiPageBody (at Page.tsx:35)
    in div (created by EuiPage)
    in EuiPage (at Page.tsx:34)
    in Page (at Simulation.tsx:80)
    in Simulation (at App.tsx:64)
    in Route (at App.tsx:63)
    in Body (at App.tsx:59)
    in Switch (at App.tsx:58)
    in div (created by EuiFlexItem)
    in EuiFlexItem (at App.tsx:57)
    in div (created by EuiFlexGroup)
    in EuiFlexGroup (at App.tsx:53)
    in Router (created by BrowserRouter)
    in BrowserRouter (at App.tsx:52)
    in App (at src/index.tsx:9)
    in StrictMode (at src/index.tsx:8)

Kibana Cross Issues
Add any Kibana related issues here.

Checklist

Delete any items that are not applicable to this issue.

  • every related Kibana issue is listed under Kibana Cross Issues list
  • kibana cross issue tag is associated to the issue if any kibana cross issue is present
@flavio-kadata flavio-kadata added the bug Something isn't working label Jun 24, 2020
@RSWilli
Copy link

RSWilli commented Jun 25, 2020

same problem here, the chart just flashes for a second and then disappears

@nickofthyme
Copy link
Collaborator

nickofthyme commented Jun 25, 2020

Thanks for bringing this to our attention. I tested out your chart code with some sample data on this codesandbox.

https://codesandbox.io/s/cocky-hypatia-ivh3p?file=/src/App.tsx

The only thing I changed from your code was the Date type to number (aka in ms) and the chart appears to render fine.

export interface SimulationStatsGraphSeries {
  id: string;
  name: string;
-  data: Array<[Date, number]>;
+  data: Array<[number, number]>;
}

If you could provide us with a small example dataset to reproduce the issue that would help a lot.

@flavio-kadata
Copy link
Author

Thanks for bringing this to our attention. I tested out your chart code with some sample data on this codesandbox.

https://codesandbox.io/s/cocky-hypatia-ivh3p?file=/src/App.tsx

The only thing I changed from your code was the Date type to number (aka in ms) and the chart appears to render fine.

export interface SimulationStatsGraphSeries {
  id: string;
  name: string;
-  data: Array<[Date, number]>;
+  data: Array<[number, number]>;
}

If you could provide us with a small example dataset to reproduce the issue that would help a lot.

Hi @nickofthyme
Thank you for taking the time.

I think I managed to create a non-functional example in codesandbox. I mean, this gives me the same error I am getting, but differently than my actual app, it plots the graph (my app doesn't):

https://codesandbox.io/s/throbbing-waterfall-99mlz?file=/src/App.tsx

Important differences:

  • I created a parent/child componente relantiopnship
  • Notice that I don't have the data ready to pass to the child as a prop, instead I call a function with some parameters and then this function should return the correct data set (this is important because as I undesrtand from the other issues related to thei error message I thik the problem occurs when the child component calls prop.someFunction() while in a render lifecycle state)
  • I added a mock useEffect looking for updates in a random attribute...
  • I also upgraded your example to React 16.3.1 (was 16.3.0)

So what I think the problem is:

  • The useEffect will dispatch the render phase
  • In this phase I'll need to re-render the Graph component
  • This component is a child that depends on the data prop
  • This prop is calculated from a function in the parent component

I think that is the scenario. Somehow React started given this warning since version 16.3.x... The one thing I don't understand is why I don't get the graph to plot, since it looks like that in the example even though I got the warning it got plotted.

Take a look at this issue: facebook/react#18178 (comment)

@nickofthyme
Copy link
Collaborator

nickofthyme commented Jun 26, 2020

Alright, so I dug a little deeper with your example, thanks for that btw.

First of all, I ran into a memory leak issue in your codesandbox example. The setInterval was being called, and subsequently doubled, on each render so after a minute there were +9k intervals running and overflowed the stack. Not sure if that was just in your example but thought I'd let you know.

Another thing I noticed is the simulation count was off. I have recently come across an issue using hooks for incrementing where the value of the useState hook (i.e. simulation) is not always updated to the most current value. The easiest way to get around this is to use the callback rather than value, like this 👇.

setInterval(function() {
-    const _simulation = simulation + 1;
-    setSimulation(_simulation);
+    setSimulation((s) => s + 1);
  }, 5000);

Now back to the error you were getting...


That comment from the react error was pretty useless, but reading down the thread I found a link to a fix on another repo final-form/react-final-form#751 (comment), that was fixed in this PR (https://github.com/final-form/react-final-form/pull/766/files). They say the issue was because of...

Summary: useField() was doing a quick subscribe-and-unsubscribe of the field on first render to get the initial state

So what I gather after looking into several threads, is that it appears any update to the component state is what triggers the issue. Why?? I'm not sure, this error only seems to have surfaced in [email protected]. Using [email protected] seems to render no issues.

I'll talk to the team tomorrow and see if we can find a solution.

Here is the working example
https://codesandbox.io/s/epic-lehmann-4idwx?file=/src/App.tsx

@flavio-kadata
Copy link
Author

The one thing I stil don't understand is why is the graph plotted in the codesandbox example, even though we've got the warning, but not in my app (graph should be in the selected area):

Bildschirmfoto vom 2020-06-26 11-36-59

At the end of the day I kind of don't mind the warning message in the console so long I get the graph. (which is happening in the codesandbox, warning+graph)

@nickofthyme
Copy link
Collaborator

Yeah that's strange. We found a fix to the console error on our end so hopefully, that will solve the visibility issue as well.

I'll put up a PR soon.

@nickofthyme
Copy link
Collaborator

Ok I put up the PR #723, in case you were interested the problem was calling a redux action/reducer in a useEffect hook when parsing the chart, commit.

Once merged we'll release a new version.

I also change our development to use the latest version of react to catch this in the future.

@flavio-kadata
Copy link
Author

Ok I put up the PR #723, in case you were interested the problem was calling a redux action/reducer in a useEffect hook when parsing the chart, commit.

Once merged we'll release a new version.

I also change our development to use the latest version of react to catch this in the future.

Awesome! Thank you so much for the making this happen so fast!

nickofthyme added a commit that referenced this issue Jun 29, 2020
markov00 pushed a commit that referenced this issue Jun 29, 2020
## [19.6.2](v19.6.1...v19.6.2) (2020-06-29)

### Bug Fixes

* react/redux issue with specParser ([#723](#723)) ([f9c29ec](f9c29ec)), closes [#720](#720)
@markov00
Copy link
Member

🎉 This issue has been resolved in version 19.6.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@markov00 markov00 added the released Issue released publicly label Jun 29, 2020
@flavio-kadata
Copy link
Author

This issue has been resolved in version 19.6.2

The release is available on:

Your semantic-release bot

Just an update on this:

  • I've updated @elastic/charts to 19.6.2
  • Funny thing is that I had to also yarn add redux-immutable-state-invariant redux-logger in order to get the app to compile... maybe missing dependencies in that specifc version? Or something on my end? (I've just changed the version in package.json and then ran yarn install)
  • Error message is gone!
  • ...but I still get no graph... anyway, probably something on my end...

Thank you

@nickofthyme
Copy link
Collaborator

nickofthyme commented Jun 29, 2020

Yup sorry about that use 19.6.3 to fix the yarn issues

@nickofthyme
Copy link
Collaborator

Let us know if that solves your visibility issues.

@flavio-kadata
Copy link
Author

Let us know if that solves your visibility issues.

Hi @nickofthyme

sorry for the delay...

Just to let you know that I'm still having issues with visibility. One thing I noticed wa this:

<div class="echChartStatus" data-ech-render-complete="false" data-ech-render-count="0"></div>

Render complete === false??? Maybe that's a hint...

What I've tryied so far:

  • Completly removed node_modules and installed everything again
  • Tryied to included some hard coded series in my chart component
  • Tryied to include a Chart component in the first steps of my app, in a way that it did not dependend on passing props, or waiting for React hooks... A very simple and hard coded compoente
  • Tryied to copy the HTML generated by your codepen and past it in my Dev Tools... Surprisingly it didn't worked and the legend didn't even got styled....

So, any ideas?

If you want to have a look at the code in my repo, here it go:

https://github.com/ffknob/elastic-apm-demo/blob/master/app/frontend/src/Simulation/components/Results/Stats/SimulationStatsGraph.tsx

Thank you!

@ffknob
Copy link

ffknob commented Jul 9, 2020

Anyway, since this is closed because the original issue was solved, I opened another one:
#747

AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this issue Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released Issue released publicly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants