search

cujojs / meld

AOP for JS with before, around, on, afterReturning, afterThrowing, after advice, and pointcuts
Other
644 stars 65 forks source link

`addAspectToMatches` in ECMAScript 2015 classes #35

Open mgechev opened 9 years ago

mgechev commented 9 years ago

Hello guys,

meld looks great! Congratulations for the job you did here.

I'm working on AOP library which takes advantage of ECMAScript 2016 decorators and as its foundation, currently, I'm using meld. However, I noticed that when applying aspect over a class defined with the ECMAScript 2015 class syntax the addAspectToMatches method doesn't work. This is caused by the fact that the methods added to the prototypes of the ES2015 classes are not enumerable but addAspectToMatches is using for..in in order to get them.

What I did is to use Object.getOwnPropertyNames instead, which works pretty well. Do you think it makes sense to do this change the algorithm used for iteration over the prototype's methods in meld?

briancavalier commented 9 years ago

Hey @mgechev, sounds like a cool project, and great catch. We haven't updated meld to be friendly for post-ES5 yet, but it's clearly time!

One tricky situation is whether to deal with inherited properties or not. Currently, meld doesn't check hasOwnProperty, and so it will happily advise inherited methods. At the time, that seemed like the right choice for AOP, which tends to be invasive by nature :)

One possibility is to limit it to own prototype properties for v2.0.

Any thoughts on whether it's a good idea to continue supporting inherited properties, and on how to support them in post-ES5?

mgechev commented 9 years ago

I think it makes sense to support inherited properties. In angular-aop I added a configuration property, which enables/disables this behavior.

Another thing, which I think makes sense in JavaScript are asynchronous joint points. In angular-aop I implemented afterResolve, beforeResolve, etc. Basically I'm adding after, before advices to promises. I'm not sure how correct is this according to the AOP theory but it comes quite handy. Do you think we should open another issue for discussion of this?

briancavalier commented 9 years ago

I think it makes sense to support inherited properties. In angular-aop I added a configuration property, which enables/disables this behavior.

Are you traversing the prototype chain manually, calling Object.getOwnPropertyNames at each level? Just curious what approach you've taken to support inheritance, given the non-enumerable ES6 method situation. Appreciate any insight on that :)

Another thing, which I think makes sense in JavaScript are asynchronous joint points

Cool. We had done that in wire.js's AOP plugin. It turned out to be incredibly useful for connecting asynchronous components.

I'd certainly be open to meld supporting it directly. Is that something you'd like to contribute?

mgechev commented 9 years ago

In angular-aop I'm using simple for..in loop, in case the user has explicitly stated that he wants the inherited methods to be traversed I just skip the call to hasOwnProperty.

I'd love to contribute! I have a few high-priority side projects which will probably keep me busy next 1-3 months. Right after that I'll follow up.