Shopify monitor that utilizes redis for caching each product This is just a simple monitor for shopify products for an array of sites. It involves parsing the /products.json of the site, checking if each product ID is already stored as a key in redis and if it isn't it needs to notify a group of discord members (haven't implemented yet). Really appreciate any feedback given!

const axios = require('axios'),
      fs = require('fs'),
      redis = require('redis');

const client = redis.createClient();

let trackingF = fs.readFileSync("../tracking.json");
let proxiesF = fs.readFileSync("../proxies.json");

let keywords = JSON.parse(trackingF).keywords.map(x =&gt; {
     return x.toLowerCase();
});

let sites = JSON.parse(trackingF).sites.map(x =&gt; {
    return x.toLowerCase();
});

let proxies = JSON.parse(proxiesF).proxies;

const getProducts = (url) =&gt; {
    return new Promise((resolve, reject) =&gt; {
        axios.get(`${url}/products.json`, rProxy()).then(response =&gt; {
           resolve(response.data.products);
        }).catch(err =&gt; {reject(err)});
    });
 }

const cache = (product) =&gt; {
    client.get(product.id, (err, reply) =&gt; {
       if (err) console.log(err);
       if (!reply) {
           client.set(product.id, JSON.stringify(product));
           console.log("NEW PRODUCT: " + product.id);
           return;
       }
    });
}

const rProxy = () =&gt; {
    return proxies[Math.floor(Math.random() * Math.floor(proxies.length))];
}

const checkStock = () =&gt; {
    console.log("[MONITOR] CHECKING");
    for (let i = 0; i &lt; sites.length; i++) {
        getProducts(sites[i]).then(products =&gt; {
           products.forEach(product =&gt; {
               cache(product)
           });
        }).catch(err =&gt; console.log(err));
    }
}

setInterval(() =&gt; checkStock(), 5000);
const axios = require('axios'),
      fs = require('fs'),
      redis = require('redis');


Personally, I discourage the use of a single var/let/const for declaring a variable. Firstly, they're not portable. Let's say you want to move fs somewhere else. Instead of copy-pasting the entire line, you end up copying that line and tacking on var/let/const anyways.

Next, say you already had redis. if you wanted to add axios, the Git changelog will look like:

- const redis = require('redis');
+ const axios = require('axios'),
+       redis = require('redis');


or this:

- const redis = require('redis');
+ const redis = require('redis'),
+       axios = require('axios');


Instead of just this:

+ const axios = require('axios');
  const redis = require('redis');


or this:

  const redis = require('redis');
+ const axios = require('axios');


You never touched redis, yet it's highlighted in the commit. This is because you had to add the ,. This ends up being unnecessary noise in the commit, and annoying in code reviews and pull requests.



Next up, I recommend a const-first policy. The priority should be const first, then let if the variable has to mutate (e.g. loop counters). Then var if you absolutely have to have a function-scoped variable defined in some nested block for whatever reason.

This block-scoped variables reduce cognitive overhead, avoiding having to remember that what variables exist in the upper scopes, and to avoid accidentally clobbering a variable elsewhere. Small change in process, but has big benefits down the line.



Node.js has native support for async/await. So you no longer have to deal with the clunky syntax of promises.

Additionally, if you have functions that use the Node-style callback API (an async function that takes a callback whose first argument is an error and the second being the result), Node has utils.promisify() which returns a promise-based version of it.



for (let i = 0; i &lt; sites.length; i++) {
    getProducts(sites[i]).then(products =&gt; {
       products.forEach(product =&gt; {
           cache(product)
       });
    }).catch(err =&gt; console.log(err));
}


You can replace this with an array.map() and Promise.all(). Put the promises returned by getProducts() in an array, pass that array to Promise.all() which returns a promise, await that promise to listen for the completion.



setInterval(() =&gt; checkStock(), 5000);


I'd wait for checkStock() to complete first before launching the next run. You wouldn't want to hammer the site every 5 seconds. They might not respond in time, and your requests may accumulate. You might be rate-limited or worse, blocked, before you know it.



Here's how I'd write it:

const axios = require('axios')
const fs = require('fs')
const redis = require('redis')

const trackingF = fs.readFileSync("../tracking.json")
const proxiesF = fs.readFileSync("../proxies.json")

const client = redis.createClient()
const clientGetAsync = utils.promisify(client.get)

const toLower = x =&gt; x.toLowerCase()

const keywords = JSON.parse(trackingF).keywords.map(toLower)
const sites = JSON.parse(trackingF).sites.map(toLower)
const proxies = JSON.parse(proxiesF).proxies;

const rProxy = () =&gt; proxies[Math.floor(Math.random() * Math.floor(proxies.length))]

const getProducts = async url =&gt; {
  const response = await axios.get(`${url}/products.json`, rProxy())
  return response.data.products
}

const cache = async product =&gt; {
  try {
    const reply = await clientGetAsync(product.id)
    if (reply) return
    client.set(product.id, JSON.stringify(product))
    console.log("NEW PRODUCT: " + product.id)
  } catch (e) {
    console.error(e)
  }
}

const checkStock = async delay =&gt; {
  console.log("[MONITOR] CHECKING")

  const promises = sites.map(async site =&gt; {
    try {
      const products = await getProducts(site)
      await Promise.all(products.map(cache))
    } catch (e) {
      console.error(e)
    }
  })

  await Promise.all(promises)

  setTimeout(() =&gt; checkStock(delay), delay)
}

checkStock(5000).catch(e =&gt; console.error(e))