Responsive IP Address Tracker done with Vanilla JS

Community feedback

• 

  P

  eestaniel•630

  @eestaniel

  Posted 10 months ago

  Hey Dylan,

  Good job with the challenge. I’ve reviewed the JavaScript code you shared and here are some suggestions to improve its efficiency, readability, and error handling, along with a breakdown of each change and its benefits.

  Direct DOM Element Access

  • Directly accessing DOM elements simplifies the code by eliminating the need for an intermediary object to store these references. Here’s how you can update the DOM manipulation:

  const form = document.getElementById("form");
  const ipElement = document.getElementById("ip");
  const locElement = document.getElementById("location");
  const ispElement = document.getElementById("isp");
  const timezoneElement = document.getElementById("timezone");
  
  function updateData(ip, isp, city, region, timezone, postalCode) {
    ipElement.textContent = ip;
    ispElement.textContent = isp;
    locElement.textContent = `${city}, ${region} ${postalCode}`;
    timezoneElement.textContent = `UTC ${timezone}`;
  }
  

  Simplify Initialization: Replace window.onload with Direct initMap() and getInfo() Calls, Utilizing defer

  • Since your script tag includes the defer attribute, we can remove the window.onload and call initMap() and getInfo() directly. This approach leverages the defer attribute effectively, ensuring the DOM is fully loaded before script execution:

  // beginning consts initialization here
  let map = initMap()
  
  // other functions here
  
  function initMap(defaultView = 15) {
    if (defaultView <= 0) {
      throw Error("View value can't be less than 1");
    }
    const map = L.map("map").setView([10, 10], defaultView);
    L.tileLayer("https://tile.openstreetmap.org/{z}/{x}/{y}.png", {
      maxZoom: 20,
      attribution: '&copy; <a href="http://www.openstreetmap.org/copyright">OpenStreetMap</a>',
    }).addTo(map);
    return map;
  
  // Other functions here
  getInfo()
  }
  

  Adding Error Handling for fetchData

  • Enhancing error handling in fetchData prevents silent failures and makes issues easier to debug:

  async function fetchData(url) {
      const response = await fetch(url);
      if (!response.ok) {
          throw new Error(`HTTP error! status: ${response.status}`);
      }
      return response.json();
  }
  

  Error Handling in Asynchronous Functions

  • Implementing try-catch in asynchronous functions like getInfo allows for graceful error management and improves the user experience by handling exceptions properly:

  async function getInfo(ipAddress = null) {
    let url = "https://geo.ipify.org/api/v2/country,city?apiKey=APIKEY_HERE";
    if (ipAddress) {
      url += `&ipAddress=${ipAddress}`;
    }
    try {
      const data = await fetchData(url);
      const {lat, lng, timezone, city, region, postalCode} = data.location;
      const {ip, isp} = data;
      updateData(ip, isp, city, region, timezone, postalCode);
      updateMap(lat, lng);
    } catch (error) {
      console.error("Failed to fetch IP data", error);
      alert("Failed to fetch IP data");
    }
  }
  

  While embedding API keys in frontend code is common, it exposes them to security risks. For sensitive interactions, consider managing API requests server-side with secure storage for API keys, like using .env files. This aligns with best practices and ensures data security.

  Overall good job with the challenge.

  P.S. I believe in your styles.css file, your html,body block contains both margin: 0, and margin: auto;.

  00

  Marked as helpful
• 

  Dylan Heslop•2,460

  @dylan-dot-c

  Posted 10 months ago

  Thanks a lot for the review🙏🙏, with reviews like these I'll definitely look forward to uploading more challenges and such. Also at first the code was all over the place so I had to take a break and came back to write some sensible code.

  Anyways, for the DOM and the elements object part I was actually planning to do something smart that will kinda prevent me from doing it explicitly for each element, I was thinking about like a for loop and join the properties by the keys but I didn't bother to do so.

  For the second part, I didn't realize the window.onload was unnecessary since I used the defer, cause I knew the defer was for like after the main content will load, so that makes the window.onload a little bit redundant and I guess I was using it mainly as a driver function so I'll replace it with a main() like most programming languages.

  For the 3rd part, it seems I was too lazy to do so but that's for that😅, error handling is important 😁.

  About the API keys I'm aware that having them anywhere in your frontend code will allow them to be discoverable and thereby used maliciously by others. It seems the recommended approach would be to store my APIkey on a backend/server then I would let the backend make the request for me and just send the data back that I need. Also seeing that this APIkey isn't linked to my Credit Card or any confidential data, I guess it's ok if it gets discovered. Even GitGuardian notified me about a confidential leak in my code when I pushed to GitHub.

  Overall thanks for the review hopefully I can give you the same in depth review for your challenges.

  Thanks again

  10