7
\$\begingroup\$

I use the following snippet with my browser homepage to detect whether or not I have my VPN connection turned on. If the IP timezone is different from the browser timezone, then it is most likely the case that the VPN connection is turned on.

Yes, this is a very unreliable way, but asking to improve the code "conceptually" would be too much. What I ask here is to review the syntax. For example, I'm not sure whether it is best to use .then(({browserTimezone, ipTimezone}) => ( ... or replace it with something like .then(function(detectionResult) { .... As a part of my self-education and fulfilling my perfectionism, I want to make this code as simple and concise as possible.

function detectVPN() {
    const browserTimezone = Intl.DateTimeFormat().resolvedOptions().timeZone

    return fetch(`https://ipapi.co/json`)
        .then(function(response) {
            return response.json()
        })
        .then(function(data) {
            const ipTimezone = data.timezone
            return {
                browserTimezone,
                ipTimezone
            }
        })
}

detectVPN()
    .then(({browserTimezone, ipTimezone}) => (
        document.getElementById('time-zones').textContent = browserTimezone + '—' + ipTimezone
    ))
    .catch(function(error) {  // e.g. if the ip lookup website isn't reachable
        document.getElementById('time-zones').textContent = 'error: ' + error.message
    });
<p id="time-zones"></p>

\$\endgroup\$
1
  • 2
    \$\begingroup\$ Note: In firefox, privacy.resistFingerprinting may change the timezone to UTC. \$\endgroup\$
    – tsh
    Commented Dec 17, 2024 at 6:03

2 Answers 2

5
\$\begingroup\$

Use modern JavaScript

Use async functions rather than .then and .catch callbacks.

Use arrow functions for simple returning functions.

Reduce source code noise

Avoid single use variables.

Do not pollute the global namespace if you can avoid it. In the rewrite the task is encapsulated in an Immediately Invoked Function Expression (IIFE).

Do not repeat code. You do the same DOM query twice. Get the value once and store it in a variable.

Use nested awaits to avoid storing intermediate async results.

Rewrite

There is no need for the intermediate variables as all you do is display some text. (and avoided the timezone capitalisation issue)

(async () => {
  const displayEl = document.querySelector("#time-zones");
  try {
    displayEl.textContent = 
      Intl.DateTimeFormat().resolvedOptions().timeZone + " — " +
      (await (await fetch(`https://ipapi.co/json`)).json()).timezone;          
  } catch ({message}) {
    displayEl.textContent = 'Error: ' + message;
  }
})();
<p id="time-zones"></p>

  • Note: Code is forced to use two capitalisations for the same name.

    Is it timeZone or timezone.

    US spelling has it as one word, thus camelCase would be timezone

    However in the rest of the world it is considered two words and thus should be written as timeZone

\$\endgroup\$
3
  • \$\begingroup\$ FWIW, regarding spelling: According to the Chicago Manual of Style (2017), Garner's Modern English Usage (2016) and American Heritage Dictionary (2016), it is time zones, two words. Though I live in the rest part of the world and might be wrong of course. \$\endgroup\$
    – john_m
    Commented Dec 25, 2024 at 21:40
  • \$\begingroup\$ If I understand correctly, the space in async () is by intent? \$\endgroup\$
    – john_m
    Commented Dec 25, 2024 at 21:45
  • \$\begingroup\$ @john_m Yes the IIFE is async by intent. Do note that top level module execution is automatically async. If you used the above rewrite as a module there would be no need to wrap the code in the async IIFE. \$\endgroup\$
    – Blindman67
    Commented Dec 27, 2024 at 19:15
9
+100
\$\begingroup\$

I'm not sure whether it is best to use .then(({browserTimezone, ipTimezone}) => ( ... or replace it with something like .then(function(detectionResult) { ...

Using Object destructuring can allow for good simplification as long as the whole object doesn't need to be referenced later (e.g. in a return statement).

I want to make this code as simple and concise as possible.

One could also use async functions with the await keyword to simplify the promise callback structures. See the code below for an example of those being applied. This helps avoid Callback-Hell and reduce indentation levels of many lines.

It is interesting that a template literal is used for the URL in the call to fetch():

    return fetch(`https://ipapi.co/json`)

This isn't wrong however some might argue those should be reserved for multi-line strings or strings where string interpolation is needed.

One other interesting aspect of the original code is that it contains one and only one semi-colon. It is wise to understand Automatic Semicolon Insertion. Some JS code is transcompiled using tools like Babel in which case semi-colons are often purposefully omitted since they will be added in the output code; otherwise it is best to consistently add semi-colons to the end of all statements.

async function detectVPN() {
    const response = await fetch('https://ipapi.co/json')
    const {timezone} = await response.json()
    return {
        browserTimezone: Intl.DateTimeFormat().resolvedOptions().timeZone,
        ipTimezone: timezone
    }
}
(async function() {
    try {
        const {browserTimezone, ipTimezone} = await detectVPN()
        document.getElementById('time-zones').textContent = `${browserTimezone}—${ipTimezone}`
    } catch({message}) {  // e.g. if the ip lookup website isn't reachable
        document.getElementById('time-zones').textContent = `error: ${message}`
    }
})()
<p id="time-zones"></p>

\$\endgroup\$
0

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.