search

bcoin-org / bcrypto

JS crypto library
Other
99 stars 41 forks source link

bn: use BigInt function over literals #33

Closed tynes closed 4 years ago

tynes commented 5 years ago

Replace all BigInt literals with BigInt functions.

This will allow the Node.js application bundles to be uglified and save some space on the size of the package. All code ran from the base directory of the bcoin gitrepo.

# without these changes, cannot use uglify-es
$ bpkg bin/node -o bcoin
$ du -h bcoin
5.3M

# with these changes, uglify-es installed globally
$ bpkg bin/node | uglifyjs > bcoin
$ du -h bcoin
4.3M

# with one additional change, removing the #!/usr/bin/env node
# this breaks the parser in bpkg/vendor/uglify-es
$ bpkg -p [ uglify-es ] bin/node -o bcoin
$ du -h bcoin
3.9M

# doing a release does uglify each file after this change
# no problems with #! like seen above
$ bpkg -p [ uglify-es ] -m -o $HOME/bcoin

Using BigInt functions over literals saves ~20% on the total package size in the end. With likely bcoin and hsd releases soon, the stylistic changes may be worth it. If you prefer the style using BigInt literals, we may need to implement our own uglifyjs that has an up to date parser. This commit can be reverted in the future when JS tooling catches up with the latest language features.

Note that when building for frontend, if the parser does not support BigInt literals, it should still be possible to build if the correct environment variable is present, NODE_BACKEND=js. Noting this here so that people can find it in the future.

Closes https://github.com/bcoin-org/bcrypto/issues/32 Closes https://github.com/bcoin-org/bcrypto/issues/14

deanpress commented 4 years ago

Is this going to be merged? Our front-end application has experienced the same issues due to this (not loading in Safari) and have so far been unable to fix it with NODE_BACKEND=js.

tynes commented 4 years ago

@deanpress The node backend has been removed on the latest branch. https://github.com/bcoin-org/bcrypto/tree/torsion/lib

There isn't a way to ignore files when bundling? And bundlers still can't handle the new syntax?

tynes commented 4 years ago

Closing as this is no longer relevant

deanpress commented 4 years ago

@deanpress The node backend has been removed on the latest branch. https://github.com/bcoin-org/bcrypto/tree/torsion/lib

There isn't a way to ignore files when bundling? And bundlers still can't handle the new syntax?

Cheers!

Yes, I currently have it set up to ignore the node version by setting the env var and bundling the package using WebpackIgnorePlugin(), which works.