angular-styleguide icon indicating copy to clipboard operation
angular-styleguide copied to clipboard

Adds block about Angular's low-level functions

Open sergiocruz opened this issue 8 years ago • 15 comments

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() and angular.equals() is simply not found within Angular 2.

sergiocruz avatar Sep 04 '16 15:09 sergiocruz

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.

joshstar avatar Sep 04 '16 22:09 joshstar

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 :)

johnpapa avatar Sep 04 '16 23:09 johnpapa

I addressed some of the comments made here, and pushed a new (squashed) commit adding:

  • intro paragraph
  • code samples
  • style number
  • some other revisions

Let me know what you all think :)

sergiocruz avatar Sep 05 '16 04:09 sergiocruz

coming along well!

johnpapa avatar Sep 05 '16 04:09 johnpapa

I added another commit adding the code samples and the two additional why's.

sergiocruz avatar Sep 05 '16 04:09 sergiocruz

All righty, added another commit addressing some more of the comments including:

  • Renamed "Low-level Functions" to "Helper Functions" (I believe the meaning is clearer now)
  • Added a couple of helper fn examples in the intro paragraph so users know what type of functions we are referring to when we mention helper functions
  • Added a note advising users to understand the differences in implementation

Again, let me know what you all think :)

sergiocruz avatar Sep 05 '16 14:09 sergiocruz

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).

gkalpak avatar Sep 05 '16 17:09 gkalpak

Thank you, I just pushed another commit.

sergiocruz avatar Sep 05 '16 17:09 sergiocruz

Hey @johnpapa are there any reviews left to add before we are able to merge this in?

sergiocruz avatar Sep 08 '16 13:09 sergiocruz

I want to get Pete Bacon Darwin's eyes on this too

johnpapa avatar Sep 14 '16 02:09 johnpapa

/summon @petebacondarwin

gkalpak avatar Sep 14 '16 11:09 gkalpak

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 angular.isArray with Array.isArray is pretty trivial.

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.

angular.forEach(obj, function(value) {
  console.log(value);
});

compared to

Object.keys(obj).forEach(function(key) {
  console.log(obj[key]);
});

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.

petebacondarwin avatar Sep 14 '16 12:09 petebacondarwin

I agree with you Pete ... especially in this

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.

And also with this ...

... I would make it a very soft suggestion rather than a strong recommendation.

johnpapa avatar Sep 14 '16 16:09 johnpapa

@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 :)

sergiocruz avatar Sep 21 '16 00:09 sergiocruz

Reads better for me.

petebacondarwin avatar Sep 21 '16 06:09 petebacondarwin