0
\$\begingroup\$

I made an extension and got it to the point where it does what I want how I want. At this point, I have a little capacity to work on code refactoring, readability and such. And I would love some feedback from more senior JS devs.

The extension does multiple different and sometimes not very connected things, all aiming at helping Adobe Analytics developers with better debugging toolset, so I don't think it would be reasonable for me to ask you to really look into what the code ultimately does. Instead, I'm completely fine just getting general feedback.

I'm more after simple, low-hanging improvements. Like, I don't want to break the code into more files or reshuffle function definitions too much. Sometimes I do a lot of nesting. When needed. I like it that way. Too many vars/constants just doesn't feel elegant to me although probably is easier to read. Same about functions. I probably could break down a few larger functions, but for now I'm looking for simpler and briefer improvements that I could make. If you could just look at it and tell me things that immediately catch your eye, that would be great.

I'm also trying to follow the functional approach here. I probably still do occasional side effecting and mutations as a result of me forgetting to clean up the code after debugging, but I really try to avoid that, so if I left any of that in the code, I should totally fix it.

I have a healthy respect for performance, but I'm not obsessed with it. As long as I'm in an optimal O() for what I'm doing, I'm more worried about elegance and brevity and then - readability.

I decided to not bother with promises since almost none of the async functions there really require a lot of time. I mean, Google likes to have a lot of things async in the extensions api, but I know that they're gonna be done in the next event loop iteration in most cases, or I don't mind the extension awaiting for them even if they take longer. I don't bother with overpromising and trying to do things while waiting.

Because it's not just a function or a few that I'm looking a feedback for, but rather a file or a few, I'll just link to the repo I have it in: https://github.com/cthae/Adobe-Launch-Debugger/blob/main/js/extension-script.js

"AA" here refers to Adobe Analytics.

For the background: I'm not a JS developer by profession. Just making an open source tool that makes my debugging simpler/faster, so I'm not necessarily familiar with classical concepts a typical front-end dev team would see as obvious.

Looks like this substack wants me to add a code snippet, so I'll add just three functions here. The rest is in the repo.

document.addEventListener('DOMContentLoaded', async () => {
  loadSettings();
  deployClickListeners();
  const client = await getValuesFromClient();
  updatePage(await checkStatus(client._satellite, client.pageLoadTime, client.pvs.length, client.links.length));
});

async function getValuesFromClient(){
  const client = {};
  client.timing = JSON.parse(await getTiming());
  client._satellite = JSON.parse(await getPageVar('_satellite'));
  client.serverCalls = JSON.parse(await getAACalls());
  client.pvs = client.serverCalls.filter((url) => {
    return !/pe=lnk/i.test(url.name);
  });
  client.links = client.serverCalls.filter((url) => {
    return /pe=lnk/i.test(url.name);
  });
  client.pageLoadTime = client.timing.domContentLoadedEventEnd - client.timing.navigationStart;
  return client;
}

function deployClickListeners() {
  document.querySelectorAll("button.tablinks").forEach((button) => {
    button.addEventListener("click", switchTab);
  });
  document.getElementById("setRedirection").addEventListener("click", setRedirection);
  document.getElementById("delAllRedirections").addEventListener("click", removeAllRedirections);
  document.getElementById("newlib").addEventListener("click", evnt => {event.target.innerText=""});
  document.getElementById("blockPageUnload").addEventListener("click", blockPageUnload);
  document.getElementById("OTCheckConsent").addEventListener("click", OTCheckConsent);
  document.getElementById("OTOpenManager").addEventListener("click", OTOpenManager);
  document.getElementById("OTRejectAll").addEventListener("click", OTRejectAll);
  document.getElementById("OTAllowAll").addEventListener("click", OTAllowAll);
  document.getElementById("raccoon").addEventListener("click", loveTheRaccoon);
}

```
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

I reviewed what you asked me to review: an init function plus {getValuesFromClient, deployClickListeners}.

The init looks fine; presumably it does the right thing.

comments where appropriate

getValuesFromClient() has a vague name and is not accompanied by documentation of any kind. It uses the undocumented _satellite JSON data structure without explanation where there ought to be some.

Apparently the URLs of interest, which are never described, will sometimes contain a "pe=lnk" substring which should be filtered out of Page View counts. OK, fair enough. It's unclear why .links should come from a re-scan rather than just a length subtraction.

consistent naming

deployClickListeners() is nicely straightforward. It is almost regular enough to DRY it up by processing a simple list.

  document.getElementById("delAllRedirections").addEventListener("click", removeAllRedirections);

Consider renaming removeAllRedirections() to match the "del" naming scheme.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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