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

b-table: Filter Debounce does not work with computed properties or data attributes in the custom filter function #4150

Closed
WillGoldstein opened this issue Sep 23, 2019 · 27 comments · Fixed by mariazevedo88/hash-generator-js#17 or CloudsdaleGroup/AnimFM#3

Comments

@WillGoldstein
Copy link

@WillGoldstein WillGoldstein commented Sep 23, 2019

Describe the bug

filter-debounce no longer works if the custom filter-function attempts to access a data or computed property.

Steps to reproduce the bug

This fiddle will illustrate the issue:
https://jsfiddle.net/0jd54wpn/1/

Notice you can try it by accessing the computed property, the data property, or both. Just comment out and see the results. Comment both if statements in filterFunction to see debounce working as expected.

Expected behavior

I expect the debouncer to work no matter what you need or try to access from within the custom filter function.

Versions

Libraries:

  • BootstrapVue: 2.0.2
  • Bootstrap: 4.3.1
  • Vue: 2.6.10

Environment:

  • Device: Windows
  • OS: 10 Pro
  • Browser: Chrome
  • Version: 77.0.3865.90

Demo link

JSFiddle

Additional context

@tmorehouse tmorehouse self-assigned this Sep 23, 2019
@tmorehouse

This comment has been minimized.

Copy link
Member

@tmorehouse tmorehouse commented Sep 23, 2019

This is strange. Must have some kind of trigger in Vue during reactive checks (getter triggering a setter/getter somewhere)

I wonder if downgrading to a lower version of Vue (say 2.6.1 or similar) has this effect...

@WillGoldstein

This comment has been minimized.

Copy link
Author

@WillGoldstein WillGoldstein commented Sep 24, 2019

Tried that here and get these errors:

image

Is there another version, perhaps more recent, that you would recommend?

@tmorehouse

This comment has been minimized.

Copy link
Member

@tmorehouse tmorehouse commented Sep 24, 2019

Not sure...

@WillGoldstein

This comment has been minimized.

Copy link
Author

@WillGoldstein WillGoldstein commented Sep 24, 2019

My fault, I was loading vue js after bootstrap vue in that fiddle. Trying vue 2.6.1 like you suggested has the same effect:
https://jsfiddle.net/azorvgjk/1/

For giggles I went back to vue 2.5.10 and it doesn't work in that version either :/. Same goes for vue 2.4.0.

@tmorehouse

This comment has been minimized.

Copy link
Member

@tmorehouse tmorehouse commented Sep 28, 2019

@WillGoldstein After a bit of digging, we've discovered one thing.

Whenever a prop changes on a component, even if it isn't directly (or indirectly) used during render, causes the component to re-render (since Vue doesn't track what causes a render function to update - any data or prop change will cause a render - unlike computed props which track what changes).

Therefor, whenever the filter prop changes, even when debounced, will cause the table component to re-render (although the filtering wont be applied until after the debounce).

What we will probably do (in release v2.1.0) is:

  • add a debounce feature to <b-form-input> (and by nature <b-form-textarea>)
  • add new prop lazy to <b-form-input>(and by nature <b-form-textarea>), which updates the v-model on change/blur (PR #4169)
  • deprecate the filter debounce feature of <b-table> in favour of the input debounce

This will decouple the filter criteria value change, optimizing the render updates

@tmorehouse tmorehouse added this to To do in 2.1.0 via automation Sep 28, 2019
@WillGoldstein

This comment has been minimized.

Copy link
Author

@WillGoldstein WillGoldstein commented Sep 28, 2019

Bravo @tmorehouse , thanks for looking closer and planning a fix for a future release. Looking forward to implementing it - it’ll be a big boost in performance for us.

@tmorehouse

This comment has been minimized.

Copy link
Member

@tmorehouse tmorehouse commented Sep 28, 2019

@WillGoldstein As a workaround, you could implement your own debounce routine on an input that, once debounced, passes the filter criteria to b-table:

Using a watcher:

<template>
  <div>
    <b-form-input v-model="rawInput" type="search"></b-form-input>
    <b-table :items="items" :fields="fields" :filter="criteria"></b-table>
  </div>
</template>

<script>
export default {
  data() {
    return {
      rawInput: '',
      criteria: '',
      // ...
    }
  },
  created() {
    // Use a non-reactive property to prevent
    // unnecessary re-renders of your component
    this.$_timeout = null
  },
  beforeDestroy() {
    // Ensure that any pending timeout is cleared on destroy
    clearTimeout(this.$_timeout)
  },
  watch: {
    rawInput(newVal, oldVal) {
      clearTimeout(this.$_timeout)
      this.$_timeout = setTimeout(() => {
        this.criteria = newVal
      }, 150) // set this value to your preferred debounce timeout
    }
  }
}
</script>

Or using event on the input

<template>
  <div>
    <!-- update is the event that updates the v-model on b-form-input -->
    <b-form-input @update="handleFilter" type="search"></b-form-input>
    <b-table :items="items" :fields="fields" :filter="criteria"></b-table>
  </div>
</template>

<script>
export default {
  data() {
    return {
      criteria: '',
      // ...
    }
  },
  created() {
    // Use a non-reactive property to prevent
    // unnecessary re-renders of your component
    this.$_timeout = null
  },
  beforeDestroy() {
    // Ensure that any pending timeout is cleared on destroy
    clearTimeout(this.$_timeout)
  },
  methods: {
    handleFilter(val) {
      clearTimeout(this.$_timeout)
      this.$_timeout = setTimeout(() => {
        this.criteria = val
      }, 150) // set this value to your preferred debounce timeout
    }
  }
}
</script>
@WillGoldstein

This comment has been minimized.

Copy link
Author

@WillGoldstein WillGoldstein commented Oct 4, 2019

Thanks @tmorehouse . Unfortunately, I am having trouble getting those workarounds to work. My filter prop on my <b-table> has to hit a custom filter function, and that function has to access data properties, which brings us full circle back to the original issue. I don't believe that work around works.

I was able to confirm this by returning true right away in my custom filter function. This made the workaround behave just like the original filter-debounce prop. But, once I removed that return true and allowed the custom filter function to hit data properties, the workaround stopped working and behaved just like the original issue.

Do you have an idea/timeline on 2.1, or at least a fix for this bug?

@tmorehouse

This comment has been minimized.

Copy link
Member

@tmorehouse tmorehouse commented Oct 4, 2019

Maybe you should make your custom filter function a computed prop, which returns a new function reference when the data properties change.

Basically a factory function that creates the filter function. And store the data properties you need to access in locally scoped variables that the filter function uses (so that it does not call the Vue getter when they are accessed repeatedly when comparing each row)

@tmorehouse

This comment has been minimized.

Copy link
Member

@tmorehouse tmorehouse commented Oct 4, 2019

v2.0.3 will have some performance improvements for tables (mainly for faster render time and reduced runtime memory footprint), which may help a bit.

@tmorehouse tmorehouse added Type: Perf and removed Type: Bug labels Oct 4, 2019
@WillGoldstein

This comment has been minimized.

Copy link
Author

@WillGoldstein WillGoldstein commented Oct 4, 2019

Maybe you should make your custom filter function a computed prop, which returns a new function reference when the data properties change.

Basically a factory function that creates the filter function. And store the data properties you need to access in locally scoped variables that the filter function uses (so that they do not call the Vue getter when they are accessed repeatedly when comparing each row)

Interesting idea. I may wait to see the performance improvements in 2.0.3 first since it may be a bit of a refactor to get it right.

@tmorehouse

This comment has been minimized.

Copy link
Member

@tmorehouse tmorehouse commented Oct 4, 2019

We do the filter function concept internally in b-table when generating the internal filter function, to pre-compile the regular expression, so that the RegExpr doesn't have to be re-compiled on each call to the filter function.

@tmorehouse

This comment has been minimized.

Copy link
Member

@tmorehouse tmorehouse commented Oct 6, 2019

BootstrapVue v2.0.3 has been released, which includes some performance enhancements for tables.

@WillGoldstein

This comment has been minimized.

Copy link
Author

@WillGoldstein WillGoldstein commented Oct 6, 2019

Thanks so much @tmorehouse for letting me know!! I’ll definitely implement it ASAP.

@tmorehouse

This comment has been minimized.

Copy link
Member

@tmorehouse tmorehouse commented Oct 16, 2019

@WillGoldstein Bootstrap v2.0.4 has been released with further performance improvements for b-table/b-table-lite

@WillGoldstein

This comment has been minimized.

Copy link
Author

@WillGoldstein WillGoldstein commented Oct 16, 2019

@WillGoldstein Bootstrap v2.0.4 has been released with further performance improvements for b-table/b-table-lite

Thanks @tmorehouse !! I’ll implement it

@tmorehouse

This comment has been minimized.

Copy link
Member

@tmorehouse tmorehouse commented Oct 16, 2019

Let us know if you find any improvement.

@WillGoldstein

This comment has been minimized.

Copy link
Author

@WillGoldstein WillGoldstein commented Oct 21, 2019

Thanks @tmorehouse - just implemented and not really seeing much in the way of significant improvement for my text filters, but that's largely because I need a debouncer before I notice a big help there.

I'm sure it's going to help in other areas that may not be immediately noticeable to me.

@tmorehouse

This comment has been minimized.

Copy link
Member

@tmorehouse tmorehouse commented Oct 21, 2019

Will see if we can get a built in debouncer for b-form-input/b-form-text-area into release 2.1.0 or 2.2.0

@WillGoldstein

This comment has been minimized.

Copy link
Author

@WillGoldstein WillGoldstein commented Oct 21, 2019

That'd be pretty nice! Thanks!

@tmorehouse

This comment has been minimized.

Copy link
Member

@tmorehouse tmorehouse commented Oct 29, 2019

PR #4314 adds built-in debouncing support to b-form-input and b-form-textarea.

When enabled, user input (keypresses) will be debounced by the specified timeout, updating the v-model (via the update event). If a change event happens before the debounce timeout expires (i.e. on input blur), the v-model will update immediately.

Will be available once v2.1.0 is released

@WillGoldstein

This comment has been minimized.

Copy link
Author

@WillGoldstein WillGoldstein commented Oct 29, 2019

Sweet @tmorehouse ! This should close out this issue sort of indirectly right? Like, no more filter-debounce prop on <b-table>, and instead just use this?

@tmorehouse

This comment has been minimized.

Copy link
Member

@tmorehouse tmorehouse commented Oct 29, 2019

We will probably leave in the filter-debounce feature for backwards compatibility, but note in the documentation to use the debounce feature of <b-form-input> instead for better performance.

We may deprecate the b-table filter debounce feature, but not fully remove it.

@WillGoldstein

This comment has been minimized.

Copy link
Author

@WillGoldstein WillGoldstein commented Oct 29, 2019

Cool, makes sense! Shall I close this issue or do we want to wait until v.2.1.0 is available?

P.S. thanks for tackling this so quickly!!

@tmorehouse

This comment has been minimized.

Copy link
Member

@tmorehouse tmorehouse commented Oct 29, 2019

We'll close the issue once v2.1.0 is released.

@tmorehouse

This comment has been minimized.

Copy link
Member

@tmorehouse tmorehouse commented Nov 13, 2019

BootstrapVue v2.1.0 has been released, which has added debouncing support to b-form-input (use this instead of the filter-debounce feature of <b-table>)

@WillGoldstein

This comment has been minimized.

Copy link
Author

@WillGoldstein WillGoldstein commented Nov 13, 2019

Cheers - thanks @tmorehouse for all the hard work!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.