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

ngmin is changing things it shouldn't #85

Closed
Francisc opened this issue Mar 20, 2014 · 12 comments
Closed

ngmin is changing things it shouldn't #85

Francisc opened this issue Mar 20, 2014 · 12 comments

Comments

@Francisc
Copy link

Hello,

I have a modal service that is inspired by your implementation.

The template check looks like this in the original file:

if((+!!config.template)+(+!!config.templateUrl)!==1)

The same looks like this in the parsed file:

if (+!!config.template + +!!config.templateUrl !== 1)

That causes JSHint to throw a warning about "confusing pluses".
It touches other parts of code as well.

Is there any way to ask ngmin to just makes sure DI will work and not touch other parts of the code?

Thanks.

@btford
Copy link
Owner

btford commented Mar 20, 2014

Why are you linting code after running it through pre-processors?

@Francisc
Copy link
Author

Well why not? :) Let's say I'm checking up on you.

Still, I want to make my code minify-able, that's why I use ngmin.
I don't want it to do more or at least let me choose.

If I want to minify it, I'll use uglify.js for example.

So, is it possible to tell ngmin to just do "its job" and leave the rest for other plugins?

@btford
Copy link
Owner

btford commented Mar 20, 2014

Unless there's a real use for this behavior, I'm not interested in implementing it. If you want to submit a PR that fixes this, be my guest.

@eddiemonge
Copy link
Collaborator

@Francisc what is that code actually doing? im just curious

@Francisc
Copy link
Author

Haha, it's Brian's code and it basically checks if both template and templateUrl were specified.
Here: https://github.com/btford/angular-modal/blob/master/modal.js#L14

It's kind of cryptic, but I liked the idea so I kept it.

@Francisc
Copy link
Author

Brian: Real use for what behaviour?

@btford
Copy link
Owner

btford commented Mar 20, 2014

As in there's some distinct advantage to ngmin -> lint -> uglify over the recommended lint -> ngmin -> uglify.

If you want to verify that your code still works after ngmin and/or uglify, you should run a test suite on the code again. Linting is helpful for catching human-introduced errors but less helpful for catching errors introduced by preprocessors.

This behavior is a side effect of regenerating the code from an AST. The behavior of the code is the same.

@Francisc
Copy link
Author

grunt-contrib-jshint's README indicates it's a real use case:
https://github.com/gruntjs/grunt-contrib-jshint#linting-before-and-after-concatenating

This is not such a big issue for me though.
What bothers me is the fact that I would like to use ngmin to only make sure DI works and I can't.

@btford
Copy link
Owner

btford commented Mar 20, 2014

What bothers me is the fact that I would like to use ngmin to only make sure DI works and I can't.

Is there some other issue you're alluding to?

@Francisc
Copy link
Author

No-no, I meant "make sure minification works with DI".
As in just convert short form to long-but-safe form and don't do anything else.

ngmin does a few code minifications which aren't really useful to me.
It has side effects, besides making code minification-safe.

@eddiemonge
Copy link
Collaborator

Please try https://github.com/olov/ng-annotate. ngmin is now deprecated: #93

If your issue isn't resolved there please open an issue at https://github.com/olov/ng-annotate/issues

If you really want ngmin to fix this issue, feel free to fork it and use that.

@Francisc
Copy link
Author

Thanks. I gave up on ngmin because it wasn't "honest".
I'll try ng-annotate for my next project.

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

3 participants