Skip to content
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

addAspectToMatches in ECMAScript 2015 classes #35

Open
mgechev opened this issue Jul 24, 2015 · 4 comments
Open

addAspectToMatches in ECMAScript 2015 classes #35

mgechev opened this issue Jul 24, 2015 · 4 comments

Comments

@mgechev
Copy link

mgechev commented Jul 24, 2015

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
Copy link
Member

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
Copy link
Author

mgechev commented Jul 24, 2015

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
Copy link
Member

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
Copy link
Author

mgechev commented Jul 26, 2015

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants