Consider making userAgentIdentifier a default parameter in the hosting check() method, then a required string elsewhere.

[mrchrisadams]

In our dev docs, for the check API, the check() method is described as follows:

To check if a single domain is green hosted, you can pass the following parameters into the check function:

domain: string the website domain you want to check for green hosting. userAgentIdentifier: string Optional since v0.14.2 the name of the project, product, or app which is performing the check.

In the actual code, the implementation looks like this

function check(domain, userAgentIdentifier) {
  // is it a single domain or an array of them?
  if (typeof domain === "string") {
    return checkAgainstAPI(domain, userAgentIdentifier);
  } else {
    return checkDomainsAgainstAPI(domain, userAgentIdentifier);
  }
}

We currently pass in this userAgentIdentifier parameter all the way through the call chain, like this, in the checkAgainst API function

async function checkAgainstAPI(domain, userAgentIdentifier) {
  const req = await fetch(
    `https://api.thegreenwebfoundation.org/greencheck/${domain}`,
    {
      headers: getApiRequestHeaders(userAgentIdentifier),
    }
  );
  const res = await req.json();
  return res.green;
}

You can't actually see what the userAgentIdentifier is likely to be until right before the the API request is made, like so - see the source in our helper file

function getApiRequestHeaders(comment = "") {
  return { "User-Agent": `co2js/${process.env.CO2JS_VERSION} ${comment}` };
}

A proposal

Right now, we say the userAgentIdentifer parameter is optional in our docs, but it's actually required in our top level check() function signature - we just sort of tidy up scenarios where it's undefined, by calling getApiRequestHeaders right at the end.

I didn't realise you can pass in callable functions as default parameters with javascript, which are evalulated at call time.

With this in mind, I think it might actually be clearer and simpler to have a function signature like this, where we clearly pass in a default function to identify an API client, when it's not specified. Something like this:

function check(domain, userAgentIdentifier = identifyAPIClient() ) {
  // is it a single domain or an array of them?
  if (typeof domain === "string") {
    return checkAgainstAPI(domain, userAgentIdentifier);
  } else {
    return checkDomainsAgainstAPI(domain, userAgentIdentifier);
  }
}

And for our identifyAPIClient() we might have some code like this, which is only concerned about identifying itself, rather than building or changing request headers too.

@param {string} comment - Optional. The app, site, or organisation that is making the request.
function identifyAPIClient(comment="") {
  return `co2js/${process.env.CO2JS_VERSION} ${comment}`
}

If we did it this way:

• we'd only have one function keep track of for identifying a client, and it'd be explicit that it's optional - in both the code and docs.
• elsewhere, throughout lifecycle of the check() function we could assume that any functions it wraps can expect a string to be passed in as the identifier for the API client. Fewer things that can change to keep track are a good thing.
• we'd end up with an implemented way to support future default arguments to be added in what you might consider our 'public' API, like our check() method. This would also be implemented in way native to javascript, without needing to change the method signatures of code too drastically.

Anyway @fershad, I'm leaving this issue here to discuss next week, as it's something I realised when reading over #189, and I think it might help with handling stuff like verbose = true as a default parameter there too, along with anything else we choose in future.

[fershad]

@mrchrisadams the plan is to move towards using an options object for anything additional (like userAgentIdentifier, verbose, or anything else we add later).

We've started down that path with #189, and I've covered off how we can guide developers to use the options object to pass in optional information for a greencheck (https://github.com/thegreenwebfoundation/co2.js/pull/192/files/ccd5bc196e34a598bb6c9cbe9f0304d55d9604e1#r1554588557)

[mrchrisadams]

Thanks @fershad , that works for me.

I'm closing this, as I think your last comment addresses anything I initially raised. I realised the nifty default parameter thing after suggesting the options parameter approach discussed in #189, and TBH I'm not familiar enough to advocate hard for it, so the options approach sounds good.