Adds user agent header for requests to Green Web Foundation APIs

[sfishel18]

the version from package.json is used to inject an environment variable when esbuild runs, which the running code can read to create the correct user agent string when making requests.

fixes #181

[sfishel18]

@fershad let me know if this looks like the right direction. there were a couple of things i wasn't sure of:

1. should the requests from the node.js version of the library also include the user agent header? i went ahead and added it there but can easily revert it

2. afaict some of the unit tests in hosting.test.js are sending actual network requests. i didn't change that behavior since i'm not sure if it's intentional. but since i added logic to spy on https.get, i could easily modify it to mock out the responses if desired.

[fershad]

@sfishel18 thanks for this. I'll go through it in the next week or two with a view to merge it into the v0.14.1 release

[fershad]

@sfishel18 thanks for this one. I went through it and made some changes:

• I added an optional param (comment) to the request headers. This allows a space for users to pass in the name of their app, site, or company which can give us a better insight into where requests are coming from.
• Hitting the actual API is as designed in the tests. I have removed nock in some tests & replaced it with spying on the https.get request headers.

There's one hurdle for this update though. Right now, the package version gets written to the code when the build is run. However, when we publish to NPM we use the np package to smooth out this process.

np bumps the package version just before publishing. I believe this should be solvable by using the npm lifecycle hooks which np runs (https://github.com/sindresorhus/np?tab=readme-ov-file#npm-hooks). I've added a version script to rerun the build when the version number is bumped.

[fershad]

@mrchrisadams this PR is ready for a review when you've got a moment.

The changes made add a User-Agent header to fetch requests made from CO2.js to the Greencheck API. This will show the current version of CO2.js by default. There's an optional parameter where the user can pass in a string to represent the app, site, or company making the request.

The output of the check function remains unchanged.

For example:

import {hosting} from '@tgwf/co2'

hosting.check("google.com", "MyCoolApp").then((result) => {
    console.log(result,);
  });

This would send a fetch request to the Greencheck API with the User-Agent: co2js/0.14.1 MyCoolApp header.

Note, that when used in the browser the User-Agent header will be set to whatever the browser sets since I believe it's read-only in that context.

Happy to walk you through the code if you need.

[mrchrisadams]

Hi @fershad thanks for tagging me - this looks good, and the comments I had are small quibbles, which while being nice to address, would not stop this being merged in.

Feel free to merge in, but do please take a mo to review the comments about whether we update related issues like the pagexray one, or adding the use of jest mocks instead of just spies.

[fershad]

Thanks @mrchrisadams I've made most of those changes, including reverting the API calls to mocks. Going to let this cool for a couple of days before merging & releasing a new build.

@sfishel18 welcome to cast your eye over this as well :)

[mrchrisadams]

Thanks for making those changes @fershad particularly adding the networking mocks (I've been feeling a bit uneasy about nock, as it's a library I'm not very familiar with, and was a bit scared of changing it.)

One last thing (and this is likely a response to you saying let's let it cool a couple of days so we can view it in with fresh eyes later) is that we're passing in comment as our variable name, but really it's setting a User-Agent string.

It might make sense to just name the argument userAgent, userAgentIdentifier or something along those lines to make the purpose of this extra argument really explicit - this feels like it might be a bit clearer than comment as a variable name in a few months.

I reckon this probably is worth deciding before we merge in, but I'd leave the final decision with you.

WDYT?

[fershad]

It might make sense to just name the argument userAgent, userAgentIdentifier or something along those lines to make the purpose of this extra argument really explicit ...

Makes a ton of sense. I've renamed it to userAgentIdentifier.

[sfishel18]

@sfishel18 welcome to cast your eye over this as well :)

thanks @fershad! sorry for the delayed reply, just got back from a vacation. all looks good to me. let me know if there's anything in particular still missing here that i could look into.

[fershad]

@sfishel18 all good mate, added you to the contributors list. Thanks for helping with this change.

[sfishel18]

@sfishel18 all good mate, added you to the contributors list. Thanks for helping with this change.

my pleasure! let me know if you have any other good starter tasks top-of-mind for someone new. no worries if not, i can look over the open issues and pick one that speaks to me :)