5
\$\begingroup\$

I have the following bit of code in a service worker that checks to see it the url contains any of the values in an array and if so it breaks out of the function. As it is run for every asset on a webpage, it can be run a lot of times so I was wondering if it could be optimised in any way:

const urlsToIgnore = [
  'boldchat.com',
  'pinimg.com',
  'responsetap.com',
  'facebook.net',
  'dwin1.com',
  'bing.com',
  'google-analytics.com',
  'googletagmanager.com',
  'hotjar.com',
  'pinterest.com',
];

self.addEventListener('fetch', event => {
  // don't load assets from these urls
  for (let i = 0; i < urlsToIgnore.length; i++) {
    if (event.request.url.indexOf(urlsToIgnore[i]) > -1) {
      return;
    }
  }
  
  // .... rest of code to cache the assets and page
});
  

\$\endgroup\$
9
  • \$\begingroup\$ Do you have access to the HTML at all? Because you add an attribute that would determine if you have to cache the data or not. \$\endgroup\$
    – Technoh
    Apr 22 '21 at 12:14
  • \$\begingroup\$ unfortunately a lot of the requests are injected by google tag manager and our marketing department \$\endgroup\$
    – Pete
    Apr 22 '21 at 12:18
  • \$\begingroup\$ The current question title, which states your concerns about the code, applies to too many questions on this site to be useful. The site standard is for the title to simply state the task accomplished by the code. Please see How do I ask a good question?. \$\endgroup\$
    – BCdotWEB
    Apr 22 '21 at 12:30
  • \$\begingroup\$ @BC my bad, question edited \$\endgroup\$
    – Pete
    Apr 22 '21 at 12:32
  • \$\begingroup\$ Probable quicker as a RegExp eg /boldchat\.com|pinimg\.com|andsoon' where names are separated with |` meaning OR and \. to escape the dot. You can build the RegExp from the array of strings eg new RegExp(urlsToIgnore,join("|").replace(/\./g, "\\."),"") \$\endgroup\$
    – Blindman67
    Apr 22 '21 at 12:42
2
\$\begingroup\$

Sets can be used to achieve constant time lookup, however, it doesn't really makes a difference with a small number of elements.

The native URL() constructor could be used to parse an URL properly and easily in JavaScript. Check browser compatibility on the bottom of the URL page on MDN.

const urlsToIgnore = new Set([
  'boldchat.com',
  'pinimg.com',
  'responsetap.com',
  'facebook.net',
  'dwin1.com',
  'bing.com',
  'google-analytics.com',
  'googletagmanager.com',
  'hotjar.com',
  'pinterest.com',
]);

self.addEventListener('fetch', event => {
  // don't load assets from these urls
  const url = new URL(event.request.url);
  if (urlsToIgnore.has(url.host)) {
    return;
  }
  
  // .... rest of code to cache the assets and page
});
\$\endgroup\$
0
\$\begingroup\$

If you can use ES6 Features in your code, use a Set for constant lookup time:

const urlsToIgnore = new Set([
  'boldchat.com',
  'pinimg.com',
  'responsetap.com',
  'facebook.net',
  'dwin1.com',
  'bing.com',
  'google-analytics.com',
  'googletagmanager.com',
  'hotjar.com',
  'pinterest.com',
]);

// later
if (uslstoIgnore.has(event.request.url) { ... }

Otherwise, just use a simple object as a map:

const urlsToIgnore = {
  'boldchat.com': 1,
  'pinimg.com': 1,
  'responsetap.com': 1,
  'facebook.net': 1,
  'dwin1.com': 1,
  'bing.com': 1,
  'google-analytics.com': 1,
  'googletagmanager.com': 1,
  'hotjar.com': 1,
  'pinterest.com': 1,
};

// later
if (uslstoIgnore[event.request.url]) { ... }

Both these options offer constant time lookup, as opposed to the linear time of your current solution.

The actual performance gain depends on the size of the list with ignored items, but since this list can be added to, I'd opt for an O(1) lookup.

\$\endgroup\$
1
  • \$\begingroup\$ Unfortunately this doesn't seem to work - the urls to ignore is only part of the url (not the full url) - see test 3: jsfiddle.net/qdthkcvy/1 \$\endgroup\$
    – Pete
    Apr 23 '21 at 8:25
0
\$\begingroup\$

The substring match catches more than you (probably) intend. For example, you have bing.com in your list, which means you'll match anything such as plumbing.com, dubbing.com and so on, not to mention any URL that contains bing.com in its local-part.

I think we need to be a bit smarter about this: first, it seems we want to restrict to the hostname part of the URL, and then match on whole words ("words" here meaning the DNS hostname components - i.e. split by . only).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.