Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why Uglify? #14

Open
moll opened this issue Feb 8, 2015 · 4 comments
Open

Why Uglify? #14

moll opened this issue Feb 8, 2015 · 4 comments

Comments

@moll
Copy link

moll commented Feb 8, 2015

Hey,

Why the dependency on UglifyJS? It's a build-minification tool. Wouldn't a plain eval be sufficient as you already do after piping everything through Uglify? Especially given that the default is to not compress the code. It would definitely be faster without involving Uglify, which I've got a feeling is to blame for the 300ms+ time spent compiling a single schema. ;-)

If people wish to compress the code as they see fit, dependency injection (a.k.a pass a function in) would be a decent solution.

@atrniv
Copy link
Member

atrniv commented Aug 1, 2015

@moll , uglify was put to be able to create readable code for debugging purposes and also to remove any dead code that was generated. I will be removing it in a future release and leave it for the user to decide how he wants to compress or minify the generated validator.

@moll
Copy link
Author

moll commented Sep 16, 2015

Great. Currently I think Themis has to be the slowest validator library evar because of that. Hundreds of milliseconds per Themis.validator call because of that. Awful awful. :)

@jonasfj
Copy link

jonasfj commented Oct 27, 2015

Moving it to be an optionalDependency might be a good...

I see it as useful when compiling schemas upfront.. as one would be willing to pay for the cost...
But if loading dynamically (even in a server) 300ms per schema is a lot of overhead (slow server startup isn't nice..).

@johnrb2
Copy link

johnrb2 commented Jan 20, 2022

Is this project maintained anymore? The uglify dependency has a critical security error according to npm audit. Could we get it removed or updated? I can submit a pull request if that will help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants