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

Please include the dist files #105

Closed inod closed 1 year ago

inod commented 1 year ago

Downloading and installing npm, all the toolchain required and dependencies, and then building the dist files is wasteful, if all I want to do is to start experimenting with the dist/iife file you mention in README.md.

Is there a technical reason you don't include the dist directory?

fershad commented 1 year ago

@inod good point, thanks from bringing this up. I don't see any reason why the /dist directory cannot be committed to git as well. As you say, having those files available would let folks start using CO2.js without NPM at all.

Let's just check with @mrchrisadams if there's any reason why /dist was excluded from git originally.

mrchrisadams commented 1 year ago

hey folks, I can't remember specifically, but these are two reasons I can remember:

  1. it seemed a bit redundant to check-in generated and minified code, when CDNs already host the files for playing around with already. I've linked to some examples below
  2. Previously, I've tried to keep generated code out of projects if I can - I've found it would result in smaller and slightly easier to read commits and PRs to review. I'm not typically reading the code in dist - it's aimed at machines, and it would be easy for me to end up checking in minified, or transpiled code without knowing if it really was the same as the code in src, so not including the dist file seemed a way to avoid this error.

An example of fetching code without needing NPM

Jsdeliver is one example service that takes files from npm, and hosts them so you can refer to them in scripts in your own code without needing to use NPM directly yourself. It's also served from a CDN already.

Here's the link to the package:

https://www.jsdelivr.com/package/npm/@tgwf/co2

You can follow the links there to the (minifed) IIFE file for linking:

https://cdn.jsdelivr.net/npm/@tgwf/co2@0.10.4/dist/iife/index.js

Another one is unpkg. You can see the same release below:

https://unpkg.com/browse/@tgwf/co2@0.10.4/

And here's the same iife file: https://unpkg.com/@tgwf/co2@0.10.4/dist/iife/index.js

Does this help meet the use case listed above?

It would be trivial to add this to the docs, to make it easier to start without needing to mess with npm.

@fershad and I had already discussed creating minified and prettified versions of the files in /dist/, so that might also make using the generated files easier to lower the barrier of entry.

On checking generated code into git

This isn't a strongly held opinion about avoiding checking some generated code assets into source control.

If there are libraries that do this regularly, and they have a way we can copy to get code checked in a safe way that avoids common errors, and it addresses the issues above, it would be a nice thing for Hacktoberfest which starts in about a week.

fershad commented 1 year ago

Thanks @mrchrisadams. As you mention, the output in dist is for machines, and it would be hard to us to verify that it's been updated correctly before merging a PR as well.

@inod do the unpkg and/or jsdelivr suggestions work for you? I can put these into the docs & readme over the weekend.

inod commented 1 year ago

Thank you very much @fershad and @mrchrisadams, I was able to use the linked built js from jsdelivr successfully.

fershad commented 1 year ago

Added to the readme in 170400d, and updated on the docs site too in https://github.com/thegreenwebfoundation/developer-docs/commit/3bcf89d4ec0321acc83e18a7718e2c6877a9d7e3.