search

NoHomey / bind-decorator

The fastest automatic method.bind(this) decorator
MIT License
68 stars 11 forks source link

Unnecessary constants #8

Closed bisubus closed 5 years ago

bisubus commented 6 years ago

As I see, this commit didn't provide reasoning for adding constants.

Unless constants are exported for testing reasons (and they are not), this can be considered antipattern. There won't be any performance gain from primitive constants either.

I would suggest to remove them, this will decrease package (which is tiny already) size by almost a half.

And thanks for this utility package that just works.

MauricioAndrades commented 5 years ago

i imagine this is to reuse the configuration and not needlessly create constants every time decorator is applied. See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty withValue example on MDN.

bisubus commented 5 years ago

@MauricioAndrades These constants are primitives, there's no performance benefit in containing them in constants vs creating them each time in modern JS engines. Notice that the logic behind the descriptor didn't change.