Is this good practice for useEffect async

Question

I am creating a simple weather app to solidify my react hook knowledge. When using useEffect I keep coming up with an error when using async await functions. I looked around and found ways around to use async functions one time in use effect.

My problem is that I want to use async/await functions twice. I use navigator.geolocation to find current location and set lat and long in state. Then, once they are set run a weather api that uses the new state of lat and long. I found multiple solutions on here about how setting state waits til the next render so using the newly set state in the next fetchAPI function wont work.

Thus, I came up with this solution.

  const [lat, setLat] = useState([]);
  const [long, setLong] = useState([]);
  const [data, setData] = useState();

  useEffect(() => {
    fetchLocation(/*uses navigator.geolocation to setLong and setLat*/);

    // hacked out a way to not call fetch data until lat and long are set.
    if (typeof lat == "number" && typeof long == "number") {
      fetchWeatherData();
    }
    console.log(lat, "<= lat");
    console.log(long, "<= long");
  }, [lat, long]); 

This solution works like I wanted on localhost because it only fetches the weatherAPI when the lat and long states are set with the first function. Before useEffect would load the weatherAPI with lat and long still set to empty causing an error. I am wondering if this is correct way to go about this problem or if there are unknown side effects that I haven't found yet.

Also this warning pops up afterwards and I am not sure if how to handle it.

"src/App.js Line 37:6: React Hook useEffect has a missing dependency: 'fetchWeatherData'. Either include it or remove the dependency array react-hooks/exhaustive-deps"

EDIT: full code as requested from comments

import React, { useState, useEffect } from "react";
import WeatherDisplay from "./weather";
require("dotenv").config();

function App() {
  const [lat, setLat] = useState([]);
  const [long, setLong] = useState([]);
  const [data, setData] = useState();

  const fetchLocation = () => {
    navigator.geolocation.getCurrentPosition((position) => {
      setLat(position.coords.latitude);
      setLong(position.coords.longitude);
    });
  };

  const fetchWeatherData = () => {
    fetch(
      `${process.env.REACT_APP_API_URL}/weather/?lat=${lat}&lon=${long}&units=metric&APPID=${process.env.REACT_APP_API_KEY}`
    )
      .then((res) => res.json())
      .then((result) => {
        setData(result);
        console.log(result);
      });
  };

  useEffect(() => {
    fetchLocation();

    // hacked out a way to not call fetch data until lat and long are set.
    if (typeof lat == "number" && typeof long == "number") {
      fetchWeatherData();
    }
    console.log(lat, "<= lat");
    console.log(long, "<= long");
  }, [lat, long]); // set as empty arrays if locations don't work

  return (
    <div className="App">
      {/* data is the data that was fetched from fetchWeatherData() */}
      <WeatherDisplay weatherData={data} />
    </div>
  );
}

export default App;

Answer 1

Putting lat and long in two separate useState's makes you lose control. You better put them inside a single useState variable:

const [coordinates, setCoordinates] = useState([]); // [lat, long]

This way, the geolocation routine calls the setter only once and the useEffect hook, which will depend on [coordinates], is always triggered at the right moment with complete information.

Regarding the danger of triggering useEffect before coordinates are set, you have two possibilities:

• initialize the hook providing some default values
• put an if-guard at the start of the function run by useEffect

Regarding the missing dependency inside the hook function, please check this comprehensive answer

Answer 2

First question that arises: do you actually need these coordinates? I mean, apart from passing them to fetchWeahterData?

If not, why bother with two useEffect?

const fetchWeatherData = (position) => {
    fetch(
      `${process.env.REACT_APP_API_URL}/weather/?lat=${position.lat}&lon=${position.long}&units=metric&APPID=${process.env.REACT_APP_API_KEY}`
    )
      .then((res) => res.json())
      .then((result) => {
        setData(result);
        console.log(result);
      });
  };

  useEffect(() => {
    navigator.geolocation.getCurrentPosition((position) => {
      fetchWeatherData(position)
    });
  }, []);

If you don't teleport, you won't have to set a dependency on position. Just fetch the position once, and subsequently call fetchWeatherData with it.

Answer 3

If I were you, I'd refactor the code like the following:

const [coords, setCoords] = useState();

// get current position as soon as the component is mounted
useEffect(() => {
  navigator.geolocation.getCurrentPosition(res => {
    if (res && res.coords) {
      setCoords(coords);
    }
  });
}, []);

// fetch weather data when coords is not empty
useEffect(() => {
  if (!coords) {
    return;
  }
  fetchWeatherData(coords);
}, [coords]);

If you want to clean your code with custom hooks, it's definitely worth to take a look at this useGeolocation hook.

import { useGeolocation } from 'beautiful-react-hooks'; 

const PositionReporter = () => {
  const [geoState, { onChange }] = useGeolocation(); 
  
  onChange(() => {
    console.log('Position changed...');
  });
    
  return (
   <DisplayDemo>
     The current position is:
     {geoState.isRetrieving && (<p>Retrieving position...</p>)}
     {geoState.isSupported && geoState.position && [
       <p key={0}>Lat: {geoState.position.coords.latitude}</p>,
       <p key={1}>Lng: {geoState.position.coords.longitude}</p>
     ]}
   </DisplayDemo>
  );
};

<PositionReporter />

Answer 4

It's ok to use internal async routines in useEffect, but in this case, you should take care of cleaning/canceling tasks when components unmounting to avoid React leakage warning.

Working demo with a custom hook (Live sandbox):

import React, { useState } from "react";
import {
  useAsyncEffect,
  E_REASON_UNMOUNTED,
  CanceledError
} from "use-async-effect2";
import cpFetch from "cp-fetch";

const API_KEY = "YOUR API KEY"; // <------Change this

const getCurrentPosition = (options) => {
  return new Promise((resolve, reject) => {
    navigator.geolocation.getCurrentPosition(resolve, reject, options);
  });
};

export default function TestComponent(props) {
  const [text, setText] = useState("");

  const cancel = useAsyncEffect(function* () {
    try {
      setText("requesting coords...");
      const {
        coords: { latitude, longitude }
      } = yield getCurrentPosition();
      setText(`${latitude} : ${longitude}`);
      const response = yield cpFetch(
        `https://api.openweathermap.org/data/2.5/weather?lat=${latitude}&lon=${longitude}&appid=${API_KEY}`
      ).timeout(props.timeout);
      setText(JSON.stringify(yield response.json(), null, 2));
    } catch (err) {
      CanceledError.rethrow(err, E_REASON_UNMOUNTED);
      setText(`Failed: ${err.toString()}`);
    }
  });

  return (
    <div className="component">
      <div className="caption">useAsyncEffect demo:</div>
      <div>{text}</div>
      <button className="btn btn-warning" onClick={cancel}>
        Cancel request
      </button>
    </div>
  );
}