sindresorhus / globals

Global identifiers from different JavaScript environments
MIT License
403 stars 116 forks source link

Minify `globals.json` #198

Open dosisod opened 1 year ago

dosisod commented 1 year ago

We could make this package a lot smaller by minifying the globals.json file. Considering that this package is downloaded 53 million (!) times a week (according to NPM), making it as small as possible will save bandwidth and storage.

To start, here is the size of the package based off the main branch (only including the relevant lines):

$ npm pack
npm notice 42.2kB globals.json
npm notice package size:  9.4 kB
npm notice unpacked size: 46.6 kB

Simply by stripping white space, newlines, etc., from the JSON file we can decrease the file size by ~7.1KB:

$ npm pack
npm notice 35.1kB globals.json
npm notice package size:  9.3 kB
npm notice unpacked size: 39.5 kB

This is good, but we can do better! If we replace true and false with 1 and 0 respectively we can reduce the file size even further:

$ npm pack
npm notice 28.3kB globals.json
npm notice package size:  9.0 kB
npm notice unpacked size: 32.7 kB

This last method will require that we change the index.js file to convert 1 and 0 with true and false to maintain backwards compatibility, which might be more trouble than it is worth.

I don't know what the deployment process for this project looks like (I don't see any GitHub Action workflows or anything), but I might suggest that, at the very least, the globals.json file be minified before publishing. I would be willing to open a PR for the second method if we decide it would be worth it.

This issue also could be applicable to the builtin-modules package as well.

Thanks!

silverwind commented 1 year ago

The index.js wrapper can be removed as well. One can just set exports and main to the json file. As far as I'm aware, this works universally in all node versions and in both CJS and ESM imports.

silverwind commented 1 year ago

I guess the ideal course of action would be a index.yaml for editing which is then built into a minified index.json for packaging. I'm just not sure whether typescript definitions would work with packages that export a json file.

fregante commented 1 year ago

works universally in all node versions and in both CJS and ESM imports

https://github.com/sindresorhus/globals/blob/d887e4c9772e25c37bb326c1e591064ca64a1a63/package.json#L13-L15

In a rare exception, this package supports node 8 and up, so it's incompatible with ESM imports between Node 12 and Node 16 (where ESM is supported, but assert {type: "json"} isn't)

silverwind commented 1 year ago

You don't need a import assertion to import a module with a .json file in main/exports. The node module loader apparently does some magic so it works without.

fregante commented 1 year ago

You don't need a import assertion to import a module with a .json file in main/exports

That's not true. Also this request is off-topic so I'll stop here.

https://unpkg.com/browse/test-test-test-json-import-will-delete-this-package@0.0.0/package.json

{
  "name": "test-test-test-json-import-will-delete-this-package",
  "version": "0.0.0",
  "exports": "./package.json"
}
// index.mjs
import x from 'test-test-test-json-import-will-delete-this-package';

console.log(x);
node index.mjs 
node:internal/errors:490
    ErrorCaptureStackTrace(err);
    ^

TypeError [ERR_IMPORT_ASSERTION_TYPE_MISSING]: Module "./node_modules/test-test-test-json-import-will-delete-this-package/package.json" needs an import assertion of type "json"
    at new NodeError (node:internal/errors:399:5)
    at validateAssertions (node:internal/modules/esm/assert:94:15)
    at defaultLoad (node:internal/modules/esm/load:86:3)
    at DefaultModuleLoader.load (node:internal/modules/esm/loader:320:26)
    at DefaultModuleLoader.moduleProvider (node:internal/modules/esm/loader:195:22)
    at new ModuleJob (node:internal/modules/esm/module_job:63:26)
    at #createModuleJob (node:internal/modules/esm/loader:219:17)
    at DefaultModuleLoader.getJobFromResolveResult (node:internal/modules/esm/loader:172:34)
    at DefaultModuleLoader.getModuleJob (node:internal/modules/esm/loader:157:17)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:76:33) {
  code: 'ERR_IMPORT_ASSERTION_TYPE_MISSING'
}

Node.js v20.1.0
silverwind commented 1 year ago

Interesting, likely I wasn't aware of this as I import all my json modules in a bundler and/or typscript which supports json without assertion. So yes, converting this module to json would be a breaking change for node users.