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

Explicit injection annotations #5

Merged
merged 1 commit into from
Jul 24, 2013
Merged

Conversation

passy
Copy link
Contributor

@passy passy commented Jul 24, 2013

Because the factory pattern used in the library isn't supported by ngmin,
minification (with enabled name mangling) breaks the dependency injection. This
adds explicit annotations with the array syntax.

Because the factory pattern used in the library isn't supported by ngmin,
minification (with enabled name mangling) breaks the dependency injection. This
adds explicit annotations with the array syntax.
gsklee pushed a commit that referenced this pull request Jul 24, 2013
Add explicit injection annotations
@gsklee gsklee merged commit 56e7505 into gsklee:master Jul 24, 2013
@gsklee
Copy link
Owner

gsklee commented Jul 24, 2013

Thanks @passy! Any idea why ngmin can't handle this?

@passy
Copy link
Contributor Author

passy commented Jul 24, 2013

/ping @btford

I'm not sure if this is something that ngmin could do. It looks rather complex to me to figure out just from the AST that the return value of _storageFactory needs to be annotated.

@btford
Copy link

btford commented Jul 29, 2013

I could either explicitly look for "function calls that return injectables that are invoked by one of the methods to create a provider for a module," but I'd rather not special-case this.

In order to make ngmin fully generic, I'd have to add dynamic analysis. I'm looking into that now. :)

@btford
Copy link

btford commented Jul 29, 2013

You can track my progress here if you're interested: btford/ngmin#44

@gsklee
Copy link
Owner

gsklee commented Jul 31, 2013

@btford Cool!

egilkh added a commit that referenced this pull request May 13, 2015
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

Successfully merging this pull request may close these issues.

3 participants