diff --git a/src/components/table/helpers/mixin-filtering.js b/src/components/table/helpers/mixin-filtering.js index 78c121efafb..89a28c79c10 100644 --- a/src/components/table/helpers/mixin-filtering.js +++ b/src/components/table/helpers/mixin-filtering.js @@ -32,8 +32,9 @@ export default { return { // Flag for displaying which empty slot to show and some event triggering isFiltered: false, - // Where we store the copy of the filter citeria after debouncing - localFilter: null + // Where we store the copy of the filter criteria after debouncing + // We pre-set it with the sanitized filter value + localFilter: this.filterSanitize(this.filter) } }, computed: { @@ -66,57 +67,55 @@ export default { // Returns the original `localItems` array if not sorting filteredItems() { const items = this.localItems || [] - // Note the criteria is debounced - const criteria = this.filterSanitize(this.localFilter) + // Note the criteria is debounced and sanitized + const criteria = this.localFilter // Resolve the filtering function, when requested // We prefer the provided filtering function and fallback to the internal one // When no filtering criteria is specified the filtering factories will return `null` - let filterFn = null - if (this.localFiltering) { - filterFn = - this.filterFnFactory(this.localFilterFn, criteria) || + const filterFn = this.localFiltering + ? this.filterFnFactory(this.localFilterFn, criteria) || this.defaultFilterFnFactory(criteria) - } + : null // We only do local filtering when requested and there are records to filter - if (filterFn && items.length > 0) { - return items.filter(filterFn) - } - - // Otherwise return all items - return items + return filterFn && items.length > 0 ? items.filter(filterFn) : items } }, watch: { // Watch for debounce being set to 0 computedFilterDebounce(newVal, oldVal) { - if (!newVal && this.filterTimer) { - clearTimeout(this.filterTimer) - this.filterTimer = null - this.localFilter = this.filter + if (!newVal && this.$_filterTimer) { + clearTimeout(this.$_filterTimer) + this.$_filterTimer = null + this.localFilter = this.filterSanitize(this.filter) } }, // Watch for changes to the filter criteria, and debounce if necessary - filter(newFilter, oldFilter) { - const timeout = this.computedFilterDebounce - if (this.filterTimer) { - clearTimeout(this.filterTimer) - this.filterTimer = null - } - if (timeout) { - // If we have a debounce time, delay the update of this.localFilter - this.filterTimer = setTimeout(() => { - this.filterTimer = null - this.localFilter = this.filterSanitize(this.filter) - }, timeout) - } else { - // Otherwise, immediately update this.localFilter - this.localFilter = this.filterSanitize(this.filter) + filter: { + // We need a deep watcher in case the user passes + // an object when using `filter-function` + deep: true, + handler(newFilter, oldFilter) { + const timeout = this.computedFilterDebounce + if (this.$_filterTimer) { + clearTimeout(this.$_filterTimer) + this.$_filterTimer = null + } + if (timeout) { + // If we have a debounce time, delay the update of `localFilter` + this.$_filterTimer = setTimeout(() => { + this.$_filterTimer = null + this.localFilter = this.filterSanitize(this.filter) + }, timeout) + } else { + // Otherwise, immediately update `localFilter` with `newFilter` value + this.localFilter = this.filterSanitize(newFilter) + } } }, - // Watch for changes to the filter criteria and filtered items vs localItems). - // And set visual state and emit events as required + // Watch for changes to the filter criteria and filtered items vs `localItems` + // Set visual state and emit events as required filteredCheck({ filteredItems, localItems, localFilter }) { // Determine if the dataset is filtered or not let isFiltered = false @@ -145,21 +144,21 @@ export default { }, created() { // Create non-reactive prop where we store the debounce timer id - this.filterTimer = null + this.$_filterTimer = null // If filter is "pre-set", set the criteria - // This will trigger any watchers/dependants - this.localFilter = this.filterSanitize(this.filter) - // Set the initial filtered state. - // In a nextTick so that we trigger a filtered event if needed + // This will trigger any watchers/dependents + // this.localFilter = this.filterSanitize(this.filter) + // Set the initial filtered state in a `$nextTick()` so that + // we trigger a filtered event if needed this.$nextTick(() => { this.isFiltered = Boolean(this.localFilter) }) }, beforeDestroy() { /* istanbul ignore next */ - if (this.filterTimer) { - clearTimeout(this.filterTimer) - this.filterTimer = null + if (this.$_filterTimer) { + clearTimeout(this.$_filterTimer) + this.$_filterTimer = null } }, methods: { @@ -167,12 +166,12 @@ export default { // Sanitizes filter criteria based on internal or external filtering if ( this.localFiltering && - !isFunction(this.filterFunction) && + !this.localFilterFn && !(isString(criteria) || isRegExp(criteria)) ) { - // If using internal filter function, which only accepts string or RegExp - // return null to signify no filter - return null + // If using internal filter function, which only accepts string or RegExp, + // return '' to signify no filter + return '' } // Could be a string, object or array, as needed by external filter function @@ -210,6 +209,7 @@ export default { }, defaultFilterFnFactory(criteria) { // Generates the default filter function, using the given filter criteria + // Returns `null` if no criteria or criteria format not supported if (!criteria || !(isString(criteria) || isRegExp(criteria))) { // Built in filter can only support strings or RegExp criteria (at the moment) return null @@ -237,7 +237,7 @@ export default { // Users can ignore filtering on specific fields, or on only certain fields, // and can optionall specify searching results of fields with formatter // - // TODO: Enable searching on scoped slots + // TODO: Enable searching on scoped slots (optional, as it will be SLOW) // // Generated function returns true if the criteria matches part of // the serialized data, otherwise false diff --git a/src/components/table/table-filtering.spec.js b/src/components/table/table-filtering.spec.js index fb54cfeb2cc..a4441762368 100644 --- a/src/components/table/table-filtering.spec.js +++ b/src/components/table/table-filtering.spec.js @@ -248,12 +248,12 @@ describe('table > filtering', () => { expect(wrapper).toBeDefined() expect(wrapper.findAll('tbody > tr').exists()).toBe(true) expect(wrapper.findAll('tbody > tr').length).toBe(3) - expect(wrapper.vm.filterTimer).toBe(null) + expect(wrapper.vm.$_filterTimer).toBe(null) await waitNT(wrapper.vm) expect(wrapper.emitted('input')).toBeDefined() expect(wrapper.emitted('input').length).toBe(1) expect(wrapper.emitted('input')[0][0]).toEqual(testItems) - expect(wrapper.vm.filterTimer).toBe(null) + expect(wrapper.vm.$_filterTimer).toBe(null) // Set filter to a single character wrapper.setProps({ @@ -261,7 +261,7 @@ describe('table > filtering', () => { }) await waitNT(wrapper.vm) expect(wrapper.emitted('input').length).toBe(1) - expect(wrapper.vm.filterTimer).not.toBe(null) + expect(wrapper.vm.$_filterTimer).not.toBe(null) // Change filter wrapper.setProps({ @@ -269,20 +269,20 @@ describe('table > filtering', () => { }) await waitNT(wrapper.vm) expect(wrapper.emitted('input').length).toBe(1) - expect(wrapper.vm.filterTimer).not.toBe(null) + expect(wrapper.vm.$_filterTimer).not.toBe(null) jest.runTimersToTime(101) await waitNT(wrapper.vm) expect(wrapper.emitted('input').length).toBe(2) expect(wrapper.emitted('input')[1][0]).toEqual([testItems[2]]) - expect(wrapper.vm.filterTimer).toBe(null) + expect(wrapper.vm.$_filterTimer).toBe(null) // Change filter wrapper.setProps({ filter: '1' }) await waitNT(wrapper.vm) - expect(wrapper.vm.filterTimer).not.toBe(null) + expect(wrapper.vm.$_filterTimer).not.toBe(null) expect(wrapper.emitted('input').length).toBe(2) // Change filter-debounce to no debouncing @@ -291,7 +291,7 @@ describe('table > filtering', () => { }) await waitNT(wrapper.vm) // Should clear the pending timer - expect(wrapper.vm.filterTimer).toBe(null) + expect(wrapper.vm.$_filterTimer).toBe(null) // Should immediately filter the items expect(wrapper.emitted('input').length).toBe(3) expect(wrapper.emitted('input')[2][0]).toEqual([testItems[1]]) diff --git a/src/components/table/table-provider.spec.js b/src/components/table/table-provider.spec.js index 78fe1bfbf13..8f260941c07 100644 --- a/src/components/table/table-provider.spec.js +++ b/src/components/table/table-provider.spec.js @@ -1,5 +1,5 @@ -import Vue from 'vue' import { mount } from '@vue/test-utils' +import { waitNT } from '../../../tests/utils' import { BTable } from './table' const testItems = [ @@ -25,7 +25,7 @@ describe('table > provider functions', () => { }) expect(wrapper).toBeDefined() - await Vue.nextTick() + await waitNT(wrapper.vm) expect(wrapper.emitted('update:busy')).toBeDefined() expect(wrapper.emitted('input')).toBeDefined() @@ -59,7 +59,7 @@ describe('table > provider functions', () => { }) expect(wrapper).toBeDefined() - await Vue.nextTick() + await waitNT(wrapper.vm) expect(wrapper.emitted('update:busy')).toBeDefined() @@ -73,12 +73,12 @@ describe('table > provider functions', () => { // Should have single empty row expect(wrapper.find('tbody').findAll('tr').length).toBe(1) - await Vue.nextTick() + await waitNT(wrapper.vm) expect(doResolve).toBeDefined() doResolve(testItems.slice()) - await Vue.nextTick() + await waitNT(wrapper.vm) expect( wrapper @@ -106,7 +106,7 @@ describe('table > provider functions', () => { }) expect(wrapper).toBeDefined() - await Vue.nextTick() + await waitNT(wrapper.vm) expect(wrapper.emitted('update:busy')).toBeDefined() @@ -120,12 +120,12 @@ describe('table > provider functions', () => { // Should have single empty row expect(wrapper.find('tbody').findAll('tr').length).toBe(1) - await Vue.nextTick() + await waitNT(wrapper.vm) expect(callback).toBeDefined() callback(testItems.slice()) - await Vue.nextTick() + await waitNT(wrapper.vm) expect( wrapper @@ -151,7 +151,7 @@ describe('table > provider functions', () => { }) expect(wrapper).toBeDefined() - await Vue.nextTick() + await waitNT(wrapper.vm) expect(wrapper.emitted('update:busy')).toBeDefined() @@ -165,7 +165,8 @@ describe('table > provider functions', () => { // Should have single empty row expect(wrapper.find('tbody').findAll('tr').length).toBe(1) - await Vue.nextTick() + await waitNT(wrapper.vm) + await waitNT(wrapper.vm) // Expect busy to be updated to false expect(wrapper.vm.localBusy).toBe(false) @@ -196,7 +197,7 @@ describe('table > provider functions', () => { }) expect(wrapper).toBeDefined() - await Vue.nextTick() + await waitNT(wrapper.vm) // Always initially emits a refresh when provider used expect(wrapper.emitted('refreshed')).toBeDefined() @@ -204,12 +205,12 @@ describe('table > provider functions', () => { // Instance refresh method wrapper.vm.refresh() - await Vue.nextTick() + await waitNT(wrapper.vm) expect(wrapper.emitted('refreshed').length).toBe(2) // Root event refreshing wrapper.vm.$root.$emit('bv::refresh::table', 'the-table') - await Vue.nextTick() + await waitNT(wrapper.vm) expect(wrapper.emitted('refreshed').length).toBe(3) wrapper.destroy() @@ -233,7 +234,7 @@ describe('table > provider functions', () => { expect(wrapper.emitted('refreshed')).not.toBeDefined() - await Vue.nextTick() + await waitNT(wrapper.vm) expect(wrapper.emitted('refreshed')).not.toBeDefined() expect(wrapper.vm.localBusy).toBe(true) @@ -244,12 +245,12 @@ describe('table > provider functions', () => { // Trigger a context change that would trigger an internal _providerUpdate wrapper.setProps({ sortBy: 'b' }) - await Vue.nextTick() + await waitNT(wrapper.vm) expect(wrapper.emitted('refreshed')).not.toBeDefined() expect(callback).toBeDefined() callback(testItems.slice()) - await Vue.nextTick() + await waitNT(wrapper.vm) // Refreshed event should happen only once, even though // triggered 3 times while busy @@ -257,9 +258,9 @@ describe('table > provider functions', () => { expect(wrapper.emitted('refreshed').length).toBe(1) // Just to be sure, we wait again and re-test - await Vue.nextTick() + await waitNT(wrapper.vm) expect(wrapper.emitted('refreshed').length).toBe(1) - await Vue.nextTick() + await waitNT(wrapper.vm) expect(wrapper.emitted('refreshed').length).toBe(1) wrapper.destroy() @@ -282,7 +283,7 @@ describe('table > provider functions', () => { }) expect(wrapper).toBeDefined() - await Vue.nextTick() + await waitNT(wrapper.vm) expect(wrapper.emitted('update:busy')).toBeDefined() expect(wrapper.emitted('input')).toBeDefined() @@ -300,7 +301,7 @@ describe('table > provider functions', () => { items: provider2 }) - await Vue.nextTick() + await waitNT(wrapper.vm) expect(wrapper.find('tbody').exists()).toBe(true) expect( @@ -314,4 +315,98 @@ describe('table > provider functions', () => { wrapper.destroy() }) + + it('calls provider only once when filter is pre-set object', async () => { + let providerCallCount = 0 + const provider = () => { + providerCallCount++ + return testItems.slice() + } + + const wrapper = mount(BTable, { + propsData: { + filter: { a: '123' }, + fields: testFields.slice(), + items: provider + } + }) + + await waitNT(wrapper.vm) + + expect(providerCallCount).toBe(1) + + await waitNT(wrapper.vm) + await waitNT(wrapper.vm) + await waitNT(wrapper.vm) + + expect(providerCallCount).toBe(1) + + wrapper.destroy() + }) + + it('provider is called when filter object child property is changed', async () => { + let lastProviderContext = {} + + // We need a wrapper app to get around a "bug" in Vue test utils that + // doesn't let us change a child property in an object and update + // that prop with the same object reference + // https://forum.vuejs.org/t/vue-test-utils-watchers-on-object-properties-not-triggered/50900/11?u=tmorehouse + const App = { + data() { + return { + filter: { + a: '123' + }, + fields: testFields.slice() + } + }, + methods: { + provider(ctx) { + lastProviderContext = ctx + return testItems.slice() + } + }, + render(h) { + return h(BTable, { + props: { + items: this.provider, + fields: this.fields, + filter: this.filter + } + }) + } + } + + const wrapper = mount(App, { + attachToDocument: true + }) + + expect(wrapper.is('table')).toBe(true) + + const $table = wrapper.find(BTable) + expect($table.exists()).toBe(true) + + await waitNT(wrapper.vm) + await waitNT(wrapper.vm) + await waitNT(wrapper.vm) + + expect(lastProviderContext.filter).toEqual({ + a: '123' + }) + + // Change the filter criteria child property, but not the object reference + // `setData` recursively traverses the object and only changes the leaf values + wrapper.setData({ filter: { a: '456' } }) + expect(wrapper.vm.filter).toEqual({ a: '456' }) + + await waitNT(wrapper.vm) + await waitNT(wrapper.vm) + await waitNT(wrapper.vm) + + expect(lastProviderContext.filter).toEqual({ + a: '456' + }) + + wrapper.destroy() + }) })