-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Adds block about Angular's low-level functions #768
base: master
Are you sure you want to change the base?
Conversation
I feel it would be better if this was prefaced "If you plan on migrating to Angular 2 or using ES2015+". There are a lot of angularjs apps that follow this guide that have but have no plans on moving up to Angular 2, and browser support limits a lot of them from using ES2015 unless a polyfill or something like Typescript is used. |
|
||
*Why?*: You can replace a lot Angular's low-level functions with native ES2015+ functionality such as [`Object.assign()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign) for copying objects and the [Spread Syntax](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_operator#Copy_an_array) for copying arrays _(ie. `[...arr1]`)_. | ||
|
||
- Note: Use a utility library like [lodash](https://lodash.com/docs/4.15.0) if you need additional functionality that is not available natively in the newest versions of JavaScript. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to omit 4.15.0
from the link (to always point to the latest version).
Please add a style code for this too. See other examples. I think where you put this is fine, i dont see a section that fits it, but i do like this new topic. Perhaps style Y500 I'll read through it when I get a chance ... til then, love to get other people's feedback too. BTW - we still need a style for components :) |
Update README.md Update README.md Addressed comments made in johnpapa#768
I addressed some of the comments made here, and pushed a new (squashed) commit adding:
Let me know what you all think :) |
|
||
Angular 1.x ships with some [helper functions](https://docs.angularjs.org/api/ng/function) that are found within JavaScript natively. In such cases we should favor usage of native JavaScript functions instead of Angular's helper functions. In other cases where functionality is not found in JavaScript natively, we should turn to utility libraries such as [lodash](https://lodash.com/docs) to fill-in the gaps. Here are some examples: | ||
|
||
|Replace | With | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great examples ... net step is to put these examples as code examples, and in the format of the avoid/recommend that the guide uses.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem! I decided to use a table here as it was a lots os micro-examples, but will move to that style then (will probably trim down the examples though).
coming along well! |
Update README.md Update README.md Addressed comments made in johnpapa#768 Low level functions: added code samples and more why's
I added another commit adding the code samples and the two additional why's. |
@@ -3268,6 +3269,95 @@ Use [Gulp](http://gulpjs.com) or [Grunt](http://gruntjs.com) for creating automa | |||
|
|||
**[Back to top](#table-of-contents)** | |||
|
|||
## Low Level Functions | |||
|
|||
Angular 1.x ships with some [helper functions](https://docs.angularjs.org/api/ng/function) that are found within JavaScript natively. In such cases we should favor usage of native JavaScript functions instead of Angular's helper functions. In other cases where functionality is not found in JavaScript natively, we should turn to utility libraries such as [lodash](https://lodash.com/docs) to fill-in the gaps. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be (somehow) clarified that not all. functions listed in the linked docs section are to be avoided. E.g. module
, injector
, bootstrap
, element
are fine to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a very good point. I wonder how to balance that with the fact that if one ever wants to upgrade to Angular 2, he or she will have to get rid of these method calls anyway.
Just mentioning this since eventual upgrade to Angular 2 is one of the why's.
Update README.md Update README.md Addressed comments made in johnpapa#768 Low level functions: added code samples and more why's Angular Helper Functions: more reviews in johnpapa#768
All righty, added another commit addressing some more of the comments including:
Again, let me know what you all think :) |
|
||
- Avoid using Angular helper functions such as `angular.copy()`, `angular.extend()` or even `angular.isUndefined()`. | ||
|
||
*Why?*: You can replace a lot Angular's helper functions with native ES2015+ functionality such as [`Object.assign()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/assign) for copying objects and the [Spread Syntax](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_operator#Copy_an_array) for copying arrays _(ie. `[...arr1]`)_. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a lot _of_ Angular's
Another reason you might want to mention is: These helpers are exposed publicly for convenience, but they are primarily intended for Angular's internal use. What this means is that, while they are optimized for Angular's needs, they may not handle all edge-cases (or handle specific - not interesting to Angular - cases sub-optimally). |
Update README.md Update README.md Addressed comments made in johnpapa#768 Low level functions: added code samples and more why's Angular Helper Functions: more reviews in johnpapa#768 Angular Helper Functions: addressing more comments on johnpapa#768
Thank you, I just pushed another commit. |
Hey @johnpapa are there any reviews left to add before we are able to merge this in? |
I want to get Pete Bacon Darwin's eyes on this too |
/summon @petebacondarwin |
While I would be delighted if we had never exposed our "helper" functions to application developers in the first place, because it would make our maintenance of the project easier, I am still not clear how it benefits application developers not to use them. Sure, if you are going to upgrade to Angular 2 then you would need to change to alternative helpers. But compared the the significant changes that will need to be made across the board for an app migration, such as completely reworking any directives that directly manipulate the DOM, I think that the cost of swapping out things like Also, I am not entirely convinced that there is much of a performance saving in many cases either. Often the helper function delegates to the native method if it is available anyway. Moreover, as is pointed out by @gkalpak, the helpers often work differently to their native counterparts, which often leads to simpler application code. E.g.
compared to
The only reason that I feel holds water is that using standard native JavaScript functions rather than Angular helpers allows more idiomatic JavaScript code that is easier for a non-Angular developers to follow. So I guess my opinion is that I am not against people using native methods if they like, but I don't feel strongly that people should always avoid using them. So I have no problem with this being added to the style guide but personally I would make it a very soft suggestion rather than a strong recommendation. |
I agree with you Pete ... especially in this
And also with this ...
|
Update README.md Update README.md Addressed comments made in johnpapa#768 Low level functions: added code samples and more why's Angular Helper Functions: more reviews in johnpapa#768 Angular Helper Functions: addressing more comments on johnpapa#768 Helper functions: Changed language to sound more like a soft suggestion
796d259
to
d2c6568
Compare
Getting the latest
@johnpapa I pushed a new commit changing the language a bit to make it sound more like a soft suggestion with examples. Hopefully this is better, but still open to additional suggestions :) |
Reads better for me. |
Angular 1.x low-level functions should be avoided in favor of language features that are available natively in its newest versions (ES2015+). For most other cases a utility library like lodash will do wonders as well.
Although it was not mentioned in the guide, avoiding low-level also helps when upgrading to Angular 2. With the help of TypeScript and newest language features, Angular 2 is able to shift more responsibility to the language itself. Therefore functions like
angular.copy()
,angular.isUndefined()
andangular.equals()
is simply not found within Angular 2.