In the example I provided, one of the two buttons will preserve the state updating only the desired part, while the other would behave like your app; you can refer to that if you want inspiration.
Or, you can think about how you would do it and come up to a solution yourself: this has nothing to do with react, but Javascript itself. 
Is it possible for a button to trigger two fetch calls, where one preserves the state and updates the desired part (fetchCurrentWeather) and the other assigns a the returned forecast data to the target, this.state.data?
I tried to incorporate your example into my handleClick() function but I’m still getting TypeErrors, one in CurrentWeather and another in Forecast:
handleClick() {
const {search_query} = this.state;
if(!search_query) return;
this.setState({loading: true, });
fetchCurrentWeather(search_query)
.then((currentWeatherData) => {
this.setState({
data: currentWeatherData
})
console.log(this.state.data.currentWeatherData);
console.log(this.state.loading);
})
fetchForecast(search_query)
.then((returnedData) => {
this.setState({
data: Object.assign(this.state.data, {forecastData: returnedData,})
})
console.log(this.state.data.forecastData);
console.log(this.state.loading);
})
this.setState({loading: false,});
}
OK. If I were looking at that, and I had to design it…
Should there just be one weather card? I think that is a judgement call. Certainly all the forecasts should be the same component.
But could they be combined with the current? Maybe. It’s almost the same layout and information. You could pass in a boolean flag like isCurrentWeather and have a few of those things render different data and toggle the font sizes.
Is that a good idea? Again it’s kind of a judgement call. The question I would be asking myself is: Are there plans in the near future to expand the data in the cards and how much do we expect the data and layout of the two cases to diverge or will they look almost the same? Sometimes in development you have to be careful not to worry about the future, “gold plating” features that may happen sometime in the future, or “future proofing” for features that may or may not ever come. On the other hand, at work I’ve had to go work on components that other people started and they clearly hadn’t thought about planned expansions for that component, things that were planned in the next week.
So, I think you could argue either way, depending on what the answer to those questions are. If you can’t answer those questions, then I think the default should be whatever is simplest and makes the most sense for what is right in front of you.
OK, so your type error on line 7 of Forecast…
A lot of beginners misread this error and assume that it means that city is undefined. No, it is telling you that data is undefined. Why is data undefined?
Assuming your github is current, I see that WeatherCard.js is sending this:
<Forecast
data={data.forecastData}
/>
So, when I look at your App.js I see this:
this.state = {
search_query: '',
data: {
currentWeatherData: {},
forecastData: [],
},
loading: false
}
So, forecastData is supposed to be an array (makes sense). Then why in Forecast, are you doing:
let {data} = this.props;
let city = data.city;
That data should be an array. What imagine happening is that data is an array of objects, each a day of data. Then you would have to map over that data to create React readable components. I worry about lines like with something like:
const {data} = this.props;
const forecastDayCards = data.map(day => createForcastDayCard(day));
return (
<div>
{ forecastDayCards }
</div>
)
So, createForcastDayCard is a function that returns a component for each day, so forecastDayCards is an array of JSX components, one for each day, and React knows what to do with { forecastDayCards }. Of course, data.map(day => createForcastDayCard(day)) could be simplified to data.map(createForcastDayCard), but I wasn’t sure if you were hip to that.
Of course, instead of a function, if you have a subcomponent, you could do something like:
const {data} = this.props;
const forecastDayCards = data.map(weather => <ForcastDayCard weather={ weather } />);
return (
<div>
{ forecastDayCards }
</div>
)
or something like that. But to be honest. I would simplify that component, maybe just have it render static strings for now, and just make sure you are getting the right data in there and then figure out what the shape of the data is.
I always say that my “secret” to debugging is to start at the last point where you know the data makes sense and the first point where it doesn’t and start moving those inward. When those tow meet, that’s where your bug usually is.
This is really great advice, Kevin!
I knew that it was the actual data that was undefined, not the city. But I couldn’t figure out why.
Per @Marmiz’s response, the problem could have had something to do with the shallow merge being performed in each this.setState() call. So I refactored a bit and made it so the previous state is deeply merged with the updated version of the state.
Updated handleClick():
handleClick() {
const {search_query} = this.state;
if(!search_query) return;
this.setState({loading: true, });
fetchCurrentWeather(search_query)
.then((currentWeatherData) => {
this.setState({
data: {...this.state.data, currentWeatherData}
})
console.log(this.state.data.currentWeatherData);
console.log(this.state.loading);
})
fetchForecast(search_query)
.then((forecastData) => {
this.setState({
data: Object.assign(this.state.data, {...this.state.data, forecastData})
})
console.log(this.state.data.forecastData);
console.log(this.state.loading);
})
this.setState({loading: false,});
}
Your point about forecastData and whether it makes sense for it to be an object vs. an array is noteworthy. I just pushed the changes but I ended up keeping forecastData as an object. But can you still map over object properties like you could array elements?
When I try to map over data.forecastData, the data is undefined. But if I try something like:
# From Forecast.js
let forecast = data.forecastData; #This logs from App.js line 53 just fine
return (
<div>
{city}
{forecast}
</div>
)
I get no errors, data.city renders like normal but data.forecastData doesn’t render anywhere (despite logging to the console from App.js #53 correctly).
I’m not sure how to debug this because I can’t see the shape of the data. I can’t access your server. You have the server in the same directory (should probably be in its own project, separating front end and backend) but I don’t have your .env file so I can’t run it. (But hiding these things is a good thing.)
So, I can’t really see what the app is seeing. I might suggest capturing the responses and create files like fixtureData/forecast.json. This would just be a json object that you collected from an actual response, basically what is being passed into the two extractData functions so people (or you) can fake the data without having to spin up the server.
Sorry! I should be trying better to include demos when I’m having problems. I cloned the project into Glitch. Would that help?
That still (afaik) requires me to spin up my own copy with my own key. But like I said, just one example each of successful responses from each of those endpoints will suffice. You can create fixture data in your app (handy for testing).
Literally just go into each extraData functions and write console.log(JSON.stringify(response)); on the first line. Cut and paste that information into a json formatter or the web and cut and paste those results into json files in your app. I usually create a folder called “fixtures”. Those are great for developing when you don’t want to keep hitting your server or the server is down, and can also be great for unit testing.
If you’re unable to do that, I may just try to spin up my own server later today.
OK, so I was able to generate some dummy data on the API web site.
So, with your App.handleClick, I think the problem was that you had two setState calls trying to manipulate the same section of state. The [docs] actually recommend that when your new state is dependent on your old state (like here) or if there are potential race conditions, you should use the functional version of setState. Here would be an implementation for currentWeatherData:
fetchCurrentWeather(search_query)
.then(currentWeatherData => {
this.setState(oldState => ({
data: {...oldState.data, currentWeatherData}
}));
})
Or more expanded for readability (I’m not sure how comfortable you are with ES6 shorthands):
fetchCurrentWeather(search_query)
.then(currentWeatherData => {
this.setState(oldState => {
const newState = { data: {...oldState.data, currentWeatherData} };
return newState;
});
});
Using this functional form of setState will prevent a race condition - at least it’s working for me now.
A few other thoughts as a scan your code…
This is what you have now:
fetchCurrentWeather(search_query)
.then((currentWeatherData) => {
this.setState({
data: {...this.state.data, currentWeatherData}
})
console.log(this.state.data.currentWeatherData);
console.log(this.state.loading);
})
fetchForecast(search_query)
.then((forecastData) => {
this.setState({
data: Object.assign(this.state.data, {...this.state.data, forecastData})
})
console.log(this.state.data.forecastData);
console.log(this.state.loading);
})
this.setState({loading: false,});
There are a few things here that I don’t think are doing what you think they are doing. For example, those console statements on lines 6 and 7? They are going to run before the preceding setState is done. If you want to see what they look like after the setState is run, you can provide a callback as a second parameter after setState:
fetchCurrentWeather(search_query)
.then(
(currentWeatherData) => {
this.setState({
data: {...this.state.data, currentWeatherData}
},
() => {
console.log(this.state.data.currentWeatherData);
console.log(this.state.loading);
}
)
And that last line, that is not going to wait for those fetch calls, but is going to run instantly. Remember that fetch is async. (Yes, I know it’s hard to learn to think this way, but it is essential to learning JS.) That should be inside the callback to the fetch calls, it could even be in the setState there that assigns the data.
But that brings up another problem with your loading flag - you have two different async processes. One solution would be to have two flags. Another solution would be instead of a flag to have a count of current fetch processes. You should have a state var like activeFetches. It starts at 0, and gets incremented when a fetch starts and gets decremented when one ends. If it is > 0, you know you are still waiting on something.
Some other nitpicky stuff (if this were at work and I was reviewing your code in a PR)…
Your component Current_Weather.js should be CurrentWeather.js by convention. Also, “search_data” should be searchData.
I would move your state out of App.js and into CurrentWeather.js. As you build up app sizes, App.js is going to get clogged up with redux stuiff, navigation/routing stuff, etc. State should be kept in the first common ancestor of whomever needs it. And remember that every component can have state. Not that you need that here, but that messed me up for a while when I was learning.
I think handleClick and handleChange are kind of a vague names for methods, especially in a component as busy as this. But others may disagree. But someone that expands that component later may get lazy and not give them more meaningful names.
Are you familiar with functional components (sometimes also called stateless functional components or SFCs)? They are a little more lightweight than class components. I think it’s a good idea to use SFCs anywhere that you don’t need state or lifecycle methods, which (as your app is now) is everywhere but App.js. The docs. There are even ways to avoid class components when you do need those things now, but baby steps.
I think that if(data.temp !== undefined) { looks a little clunky. I would have done if(data.temp) {, checking if it’s truthy. Of course, there is the odd chance that the temp is 0 Kelvin, but I think we can risk it. 
And personally, rather than nesting all that stuff up one level and have your return null; at the end, I’d rather do it at the beginning: if (!data.temp) return null; Then your other return can follow without having to be nested. Unnecessary nesting makes code harder to read.
I would also recommend using a linter, like ESLint with the Air B&B settings (probably the default). They drive you crazy for a while as they nag about spaces and formatting, but you end up writing in a much more uniform style, and that is almost an industry standard now - it will make your life so much easier if you get a job,
But don’t get me wrong, there is some good stuff here. You’re definitely on your way.
I can’t begin to appreciate your help with this. At the end of the day, I just want to be better at React development. 
I admit that keeping async in mind when working with fetch is tough. I went ahead and moved the two console statements into the callback to the fetch calls.
Wouldn’t that be WeatherCard since it’s a parent to CurrentWeather and Forecast?
And then where would handleClick() and handleChange() go if the <SearchBar /> component resides in <App />?
I’ve done the tutorials about Redux on fCC but I just don’t feel confident enough to incorporate that given I’m still struggling with basic React state management.
Yes, I’ve read about SFCs but don’t use them often, to be honest (probably another bad habit built early on 
). I can see, however, some areas of my project where that would make sense. For example, <SearchBar /> seems like a good place to start?
I can’t begin to appreciate your help with this. At the end of the day, I just want to be better at React development.
It’s cool. You’re doing well. I just remember how much I struggled learning stuff like this. People helped me, and before long you’ll be helping others.
Wouldn’t that be
WeatherCardsince it’s a parent toCurrentWeatherandForecast?
Yes, I mispoke. And now that I think about it, this isn’t quite how I usually do this. Often I move App.js into /components. And I usualy have a folder for each other component, having a folder tree mimicking the component structure. But it isn’t even as simple as that - for example with reusable components. There are different ways to organize your app. Mine isn’t necessarily the best.
And then where would
handleClick()andhandleChange()go if the<SearchBar />component resides in<App />?
With your main state, in WeatherCard. Individual components may have their own state (like a button handler specific only to that screen) but it makes sense that have your main state in the common ancestor (a problem redux solves).
But now that I think about, I probably wouldn’t have had a WeatherCard component. Just take all of this with a grain of salt.
I’m not saying you should change these things - they’re just observations, things to think about.
I’ve done the tutorials about Redux on fCC but I just don’t feel confident enough to incorporate that given I’m still struggling with basic React state management.
Someone here on FCC once said something like: Don’t teach people Redux. Let them struggle with React and as their app gets bigger and bigger and state management begins to drive them crazy, then they’ll be ready for Redux. There’s some truth to that. React is such a conceptual shift from what came before. Then Redux is a big conceptual shift in and of itself. It will be frustrating. But there is a certain beauty to it once you understand it. And it does make state management much easier on large projects.
Yes, I’ve read about SFCs but don’t use them often, to be honest (probably another bad habit built early on
It’s not a big deal. It’s very easy to switch a class component to an SFC. But in general, I think SFCs are preferable to class components, unless you need something specific that only the class component can do.
And one more thing I just noticed in the code:
onClick={() => this.handleClick()}
That is a little redundant. The onClick prop just needs a function so:
onClick={this.handleClick}
You are just wrapping a call to that function in an anonymous function. Just don’t do onClick={this.handleClick()} because that would accidentally invoke the function.
Similarly:
onChange={(event) => this.handleChange(event)}
can be shortened to:
onChange={this.handleChange}
JS is smart enough to see that the onChange event is sending a parameter and will pass that to the function you give.
There are cases where you do need to wrap those handlers in anonymous functions, for example when you have to pass some variable to it other than what is already being passed by the event. For example it if was:
onChange={(event) => this.handleChange(event, someVariableLocalToTheFunction)}
In that case, you couldn’t pass in that other variable without accidentally invoking the function.
Yeah, it’s weird, but you’ll get the hang of it.
This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.