thegreenwebfoundation / co2.js

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

After update to 0.11.0 API stopped working (on node.js) #110

Closed michael-voit closed 1 year ago

michael-voit commented 1 year ago

Hey there. I tested to updated from 0.10.4 to 0.11.0. We use the npm library in a node.js tool and check domains via the API with it. With 0.10.4 it worked without any problems. With 0.11.0 I get the error "ReferenceError: fetch is not defined". So I had a look in the code and saw that with this version you changed to "fetch" in the src/hosting-api.js in lines 16 and 28.

fetch is not a part of node.js. Do I have to find a workaround on my side with node-fetch - or could that be an issue for more people than me?

mrchrisadams commented 1 year ago

hi @michael-voit

Can you let me know if you're importing the index.js or index-node.js?

We did split the two recently, so that commonjs, require pulls in the node focussed library, and the import defaults to the newer fetch API, that is in more recent versions of node hs.

You can see in the package.json here - we use the node option on line 5

https://github.com/thegreenwebfoundation/co2.js/blob/main/package.json#L5

But for import statements we use a different entry point:

https://github.com/thegreenwebfoundation/co2.js/blob/main/package.json#L9

These end up pulling slightly different configurations

https://github.com/thegreenwebfoundation/co2.js/blob/main/src/index-node.js https://github.com/thegreenwebfoundation/co2.js/blob/main/src/hosting-node.js

With import, we use the fetch API: https://github.com/thegreenwebfoundation/co2.js/blob/main/src/indexjs https://github.com/thegreenwebfoundation/co2.js/blob/main/src/hosting-api.js

Can you let me know how how you're pulling in the library? If this isn't the case, I'm happy to investigate further

michael-voit commented 1 year ago

@mrchrisadams Thank you Chris!

We do const greenerweb = require('@tgwf/co2'); at the top of the file. So it should be the right way...

Later we do domainIsGreen[domain] = await greenerweb.hosting.check(domain);

...

michael-voit commented 1 year ago

So if I understand the concept right after your very good structured answer and a few minutes of looking in it, with pulling in the library via "require" it "starts" with index-node.js. So there should be hosting-node.js be used instead of hosting.js.

https://github.com/thegreenwebfoundation/co2.js/blob/main/src/index-node.js#L2

import hosting from "./hosting-node.js";

instead of

import hosting from "./hosting.js";

mrchrisadams commented 1 year ago

Oh cripes, good catch @michael-voit, that is indeed correct.

If you we to make a PR makign that change, we'd love to add your to the list of contributors, to put into the next patch release.

Especially given it's Hacktoberfest.

👍 ?

fershad commented 1 year ago

@michael-voit we've released a patch for this https://github.com/thegreenwebfoundation/co2.js/releases/tag/v0.11.3

michael-voit commented 1 year ago

Perfect! Thanks everyone! :) I had no time to do that myself, otherwise I would have made my first PR to this wonderful library!