thegreenwebfoundation / co2.js

An npm module for accessing the green web API, and estimating the carbon emissions from using digital services
Other
375 stars 47 forks source link

Different result for `hosting.check` depending on import type #202

Closed GJZwiers closed 2 months ago

GJZwiers commented 4 months ago

Describe the bug The result of hosting.check() is different depending on whether the package is imported as ES Module or CommonJS. Checking google.com returns false using CommonJS import but true using ESM import.

To Reproduce CommonJS code:

// co2.js
const { hosting } = require("@tgwf/co2");

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

package.json:

{
  "type": "commonjs",
  "dependencies": {
    "@tgwf/co2": "^0.14.4"
  }
}

result:

node co2.js

false

ESM Code:

// co2.js
import { hosting } from "@tgwf/co2";

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

package.json:

{
  "type": "module",
  "dependencies": {
    "@tgwf/co2": "^0.14.4"
  }
}

result:

node co2.js

true

Expected behavior The outcome of hosting.check should be equal.

Environment (please complete the following information):

Additional context (if applicable)

fershad commented 3 months ago

Hi @GJZwiers I just want to check with you that the 2nd parameter you're passing into the check function is a userAgentIdentifier identifier string?

The reason you're getting a false result for the CommonJS import would be because it is using the node specific implementation of the hosting check function (https://github.com/thegreenwebfoundation/co2.js/blob/main/src/hosting-node.js). This check function takes three parameters (domain, db, userAgentIdentifier), which is different to the ESM version of the check function which only takes two (domain, userAgentIdenitifier).

The 2nd parameter (db) allows for a local snapshot of our green domains dataset to be used for the checks (rather than hitting the public API endpoint). So when you pass in the string "Google" into the CommonJS function it thinks that's a path to the database snapshot, and so returns false because nothing is found.

I realise that this is not captured in our documentation page, and will create an issue to add it there. https://developers.thegreenwebfoundation.org/co2js/tutorials/check-hosting/

There's also some work to add functionality to these functions in #192. I will look to see what additional error handling we can have in place to mitigate cases like this.

GJZwiers commented 3 months ago

@fershad Thank you for the clarification, it appears I'm using the function with three parameters, the following code behaves similar to the ESM code and returns true as well:

const { hosting } = require("@tgwf/co2");

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

However, the types for the function that are shown on hover in my editor (VS Code) are showing only two parameters, which was a bit confusing:

function check(domain: string | string[], userAgentIdentifier?: string | undefined): boolean | string[]

I don't actually have a requirement to use the CommonJS import, but I was following along with the tutorial and ran into this.

fershad commented 2 months ago

@GJZwiers this should now be solved with the changes deployed in version 0.15 today.

GJZwiers commented 2 months ago

@fershad Thanks!