search

MikeMcl / toFormat

Adds a toFormat instance method to big.js or decimal.js.
MIT License
34 stars 11 forks source link

uglifyjs missing from dependencies list #4

Open joepie91 opened 4 years ago

joepie91 commented 4 years ago

The build script uses uglifyjs, but this is not declared in the devDependencies list. The build in the package can be exactly reproduced with uglify-js@~2.5.0, but presumably later versions of uglify-js should also result in a working build.

As an aside, it would be preferable not to include a .min.js in the package on npm at all, and either distribute it separately or let people's own bundler handle the minification step - if someone is installing the dependency through npm, chances are they already have their own build stack anyway, and including minified code in the npm package makes it more difficult to audit the dependency.

MikeMcl commented 4 years ago

uglifyjs missing from dependencies list

Yes, it's missing and should be added. As is the case with all my libraries...

As an aside, it would be preferable not to include a .min.js in the package on npm at all

Yes, I could leave it in the repo but remove it from the npm package.

Also, I could commit a package-lock.json and yarn.lock file.

joepie91 commented 4 years ago

Yes, I could leave it in the repo but remove it from the npm package.

That seems like a reasonable solution.

Keep in mind that adding an .npmignore will override the .gitignore in your repo from npm's perspective, not add to it, so you need to copy the contents of any .gitignore files into it as well. Mentioning this because it trips up a lot of people :)

Also, I could commit a package-lock.json and yarn.lock file.

That would help for reproducibility as well, yeah, at least having one of them if it's not practical to have both for every project. Thanks!