search

yaru22 / angular-json-human

Angular Json Formatting for Human Beings (inspired by https://github.com/marianoguerra/json.human.js)
MIT License
46 stars 16 forks source link

Add DI annotation for jsonHumanHelper directive #5

Closed adharris closed 10 years ago

adharris commented 10 years ago

The jsonHumanHelper directive uses angular's ability to infer dependencies, which only works when the code is not minified. This simply adds an annotation to the directive definition.

yaru22 commented 10 years ago

HI @adharris, thanks for catching that.

angular-json-human uses ngmin to add DI annotation. RecursionHelper should've been added by ngmin but it wasn't. So I investigated this further to find out why it didn't work. I believe the reason ngmin didn't work is because I manually injected $compile above so ngmin probably thought it doesn't need to add DI annotation to this file.

I'd like to resolve this by removing manual DI injection ($compile) so that ngmin works correctly again. Would you like to change the code to do that?

adharris commented 10 years ago

Oh, huh, didn't notice the ngmin. I'm actually concating and minifying separately in my build scripts. That might explain why I'm getting this issue.

yaru22 commented 10 years ago

Once the correct fix is merged, even if you are concatenating and minifying separately, if you are using dist/angular-json-human.js, you will have properly DI annotated code.

Would you like to fix it the way I suggested above? If you'd rather have me do it, let me know. Otherwise, I'll wait for your new commit before merging this PR.

adharris commented 10 years ago

Yup, you're right. Removing the $compile on the RecursionHelper made both the minified and unminified files have the proper annotations. Here's a new commit.

yaru22 commented 10 years ago

Thanks for the contribution @adharris.