Dev #1857

Merged
merged 3 commits into from Jul 2, 2016

Projects

None yet

3 participants

@ashleybrener
Contributor

IMPORTANT: for all the pull requests use the dev branch

Answer the following depending on the type of change you want to merge

Code

  1. Have you added test(s) for your patch? If not, why not?

Yes

  1. Can you provide an example of your patch in use?

Bug Report

  1. Is this a breaking change?

No

Content

Provide a short description about what you have changed:

Prior to this modification, tag instances mixed in with Classes (Functions) would not be able to access functions and properties beyond the Class's immediate prototype, that is the Class's parent class prototype and so on

@GianlucaGuarini GianlucaGuarini and 1 other commented on an outdated diff Jun 29, 2016
lib/browser/tag/tag.js
// loop the keys in the function prototype or the all object keys
- each(Object.getOwnPropertyNames(mix), function(key) {
- // bind methods to self
- if (key != 'init')
+ each(props, function(key) {
+ // bind methods to self (if not init or riot.observable functions)
+ if (!/^(init|on|off|one|trigger)$/.test(key))
@GianlucaGuarini
GianlucaGuarini Jun 29, 2016 Member

This will make this method a bit slower, and the regex should be moved outside the loop

@ashleybrener
ashleybrener Jul 6, 2016 Contributor

I originally encountered an error with the observable event handlers, but this seems to be ok.

@GianlucaGuarini
Member

@ashleybrener thank you for your patch! I need to verify it and I will be merged probably in riot@2.5.0

@GianlucaGuarini GianlucaGuarini merged commit d65c997 into riot:dev Jul 2, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@GianlucaGuarini
Member

thanks for this!

@MartinMuzatko
Member

Funny, I discovered this problem a few days too, but thought I wasn't doing it right :P

Thanks @ashleybrener !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment