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

Recursively find parent for callbacks #59

Open
stevendesu opened this issue Feb 20, 2017 · 2 comments
Open

Recursively find parent for callbacks #59

stevendesu opened this issue Feb 20, 2017 · 2 comments

Comments

@stevendesu
Copy link

I have a fairly simple component set up like so:

<template>
    <row>
        <column>
            <vuetable url="..."></vuetable>
        </column>
    </row>
</template>

<script>
    export default {
        data () => {
            return {
                fields: [{name: "test", callback: "test"}]
            };
        },
        methods: {
            test: (value) => value.split('').reverse().join('')
        }
    }
</script>

When trying to call the callback, the following code is triggered in VueTable:

    callCallback: function(field, item) {
      if ( ! this.hasCallback(field)) return

      if(typeof(field.callback) == 'function') {
       return field.callback(this.getObjectValue(item, field.name))
      }

      let args = field.callback.split('|')
      let func = args.shift()

      if (typeof this.$parent[func] === 'function') {
        let value = this.getObjectValue(item, field.name)

        return (args.length > 0)
          ? this.$parent[func].apply(this.$parent, [value].concat(args))
          : this.$parent[func].call(this.$parent, value)
      }

      return null
    },

The relevant line here is if(typeof this.$parent[func] === 'function')... Because this.$parent refers to the column, not my custom component. And of course <column> doesn't have a method called test

This is easily resolved with something like the following:

let elem = this.$parent;
while (elem.$parent && !elem[func]) {
    elem = elem.$parent;
}

This would travel up the component tree until it found one where the callback was defined. Of course performing this while loop per row could lead to some gross performance penalties, so you may want to add caching of the callback afterwards.

Just a thought for an improvement

@ratiw
Copy link
Owner

ratiw commented Feb 21, 2017

@stevendesu Thanks for you suggestions and contributions later. (Yeah, I've noticed) :) Even though, I do not have time to follow up on them at the moment, but I did read and really appreciate it.

When I first implemented this, I had a mixed feeling about it. Trying to contain everything within the component itself and exposes its functionalities to the externals via api and events.

As in using jquery, I see this as a convenient feature to automatically look up its parent for override-able function, but I'm trying not to go beyond its immediate parent as I usually don't have a need for that. And I really don't know if there will be any case that requires going up the chain or not. Because if uses incorrectly, it will be very hard to find the problem.

What's your opinion on this?

@stevendesu
Copy link
Author

Thinking about it for some time, I feel like the proper solution is instead of passing the name of the callback, the actual callback should be passed in. This way the component is handed everything it needs to operate and does not need to go looking for it in the parent.

Looking at the code, this is actually how it's written -- if a function is passed instead of a name, the function is executed. I might update the tutorial, however, to make it clear that this is possible (and also the preferred method, as it would be more performant)

https://github.com/ratiw/vuetable-2-tutorial/blob/master/doc/lesson-06.md

Currently the tutorial only demonstrates passing a string for the callback

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