Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

docs(guide/di): Use square bracket notation for $inject annotation#5104

Closed
ewinslow wants to merge 1 commit into
angular:masterfrom
ewinslow:patch-1
Closed

docs(guide/di): Use square bracket notation for $inject annotation#5104
ewinslow wants to merge 1 commit into
angular:masterfrom
ewinslow:patch-1

Conversation

@ewinslow
Copy link
Copy Markdown
Contributor

This recommendation is better as it ensures that the $inject property doesn't get minified when aggressively obfuscating code.

Re: CLA -- I work at Google.

@mary-poppins
Copy link
Copy Markdown

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@petebacondarwin
Copy link
Copy Markdown
Contributor

@ewinslow - thanks for the PR. By the way, the commit message subject should not start with a capital letter.

@petebacondarwin
Copy link
Copy Markdown
Contributor

At this line we are also using a property lookup rather than a square bracket dereference to actually get the property.
https://github.com/angular/angular.js/blob/master/src/auto/injector.js#L53

I guess you are not applying this aggressive obfuscation to the angular.js code?

jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants