From c7088749de7c6a6bd945ae54042ca2b7d2e4fd3f Mon Sep 17 00:00:00 2001 From: Justineo Date: Thu, 28 Nov 2024 14:50:04 +0800 Subject: [PATCH 1/6] fix(ktabledata): fix fetcher cache key --- src/components/KTableData/KTableData.vue | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/components/KTableData/KTableData.vue b/src/components/KTableData/KTableData.vue index a5f5fab547..ef6ef288cd 100644 --- a/src/components/KTableData/KTableData.vue +++ b/src/components/KTableData/KTableData.vue @@ -303,15 +303,7 @@ const getCellSlots = computed((): string[] => { const isInitialFetch = ref(true) const fetchData = async () => { - // @ts-ignore - fetcher is required and will always be defined - const res = await props.fetcher({ - pageSize: pageSize.value, - page: page.value, - query: props.searchInput || filterQuery.value, - sortColumnKey: sortColumnKey.value, - sortColumnOrder: sortColumnOrder.value, - offset: offset.value, - }) + const res = await props.fetcher(fetcherParams.value) tableData.value = res.data as Record[] total.value = props.paginationAttributes?.totalCount || res.total || res.data?.length || 0 @@ -377,6 +369,15 @@ const initData = () => { const previousOffset = computed((): string | null => offsets.value[page.value - 1]) const nextOffset = ref(null) +const fetcherParams = computed(() => ({ + pageSize: pageSize.value, + page: page.value, + query: props.searchInput || filterQuery.value, + sortColumnKey: sortColumnKey.value, + sortColumnOrder: sortColumnOrder.value, + offset: offset.value, +})) + // once initData() finishes, setting tableFetcherCacheKey to non-falsey value triggers fetch of data const tableFetcherCacheKey = computed((): string => { if (!props.fetcher || !hasInitialized.value) { @@ -389,6 +390,8 @@ const tableFetcherCacheKey = computed((): string => { identifierKey = props.cacheIdentifier } + identifierKey += `-${JSON.stringify(fetcherParams.value)}` + if (props.fetcherCacheKey) { identifierKey += `-${props.fetcherCacheKey}` } From 81bfa432671d4699cae61dc35a4dfd57723d6e24 Mon Sep 17 00:00:00 2001 From: Justineo Date: Thu, 28 Nov 2024 14:52:40 +0800 Subject: [PATCH 2/6] fix(ktabledata): move code up --- src/components/KTableData/KTableData.vue | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/components/KTableData/KTableData.vue b/src/components/KTableData/KTableData.vue index ef6ef288cd..bd97137938 100644 --- a/src/components/KTableData/KTableData.vue +++ b/src/components/KTableData/KTableData.vue @@ -301,6 +301,15 @@ const getCellSlots = computed((): string[] => { return Object.keys(slots).filter((slot) => tableHeaders.value.some((header) => header.key === slot)) }) +const fetcherParams = computed(() => ({ + pageSize: pageSize.value, + page: page.value, + query: props.searchInput || filterQuery.value, + sortColumnKey: sortColumnKey.value, + sortColumnOrder: sortColumnOrder.value, + offset: offset.value, +})) + const isInitialFetch = ref(true) const fetchData = async () => { const res = await props.fetcher(fetcherParams.value) @@ -369,15 +378,6 @@ const initData = () => { const previousOffset = computed((): string | null => offsets.value[page.value - 1]) const nextOffset = ref(null) -const fetcherParams = computed(() => ({ - pageSize: pageSize.value, - page: page.value, - query: props.searchInput || filterQuery.value, - sortColumnKey: sortColumnKey.value, - sortColumnOrder: sortColumnOrder.value, - offset: offset.value, -})) - // once initData() finishes, setting tableFetcherCacheKey to non-falsey value triggers fetch of data const tableFetcherCacheKey = computed((): string => { if (!props.fetcher || !hasInitialized.value) { From aeb662a969330a8808802bf65145e5785e16ec72 Mon Sep 17 00:00:00 2001 From: Justineo Date: Thu, 28 Nov 2024 17:11:08 +0800 Subject: [PATCH 3/6] fix(ktabledata): modify tests --- src/components/KTableData/KTableData.cy.ts | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/components/KTableData/KTableData.cy.ts b/src/components/KTableData/KTableData.cy.ts index 70f2eb8d1c..0b68b715c7 100644 --- a/src/components/KTableData/KTableData.cy.ts +++ b/src/components/KTableData/KTableData.cy.ts @@ -709,12 +709,13 @@ describe('KTableData', () => { cy.get('.table tbody').find('tr').should('have.length', 2) cy.get('.table tbody').should('contain.text', 'row10') cy.get('@fetcher') - .should('have.callCount', 3) + // TODO: Investigate why fetcher is called twice here + .should('have.callCount', 4) .its('lastCall') .should('have.been.calledWith', { pageSize: 10, page: 2, offset: '10', query: '', sortColumnKey: '', sortColumnOrder: 'desc' }) .then(() => cy.wrap(Cypress.vueWrapper.setProps({ fetcherCacheKey: '2' }))) // manually trigger refetch .get('@fetcher') - .should('have.callCount', 4) + .should('have.callCount', 5) .its('lastCall') .should('have.been.calledWith', { pageSize: 10, page: 2, offset: '10', query: '', sortColumnKey: '', sortColumnOrder: 'desc' }) }) @@ -767,19 +768,20 @@ describe('KTableData', () => { cy.get('.table tbody').find('tr').should('have.length', 2) cy.get('.table tbody').should('contain.text', 'row10') cy.get('@fetcher') - .should('have.callCount', 3) + // TODO: Investigate why fetcher is called twice here + .should('have.callCount', 4) .its('lastCall') .should('have.been.calledWith', { pageSize: 10, page: 2, offset: null, query: '', sortColumnKey: '', sortColumnOrder: 'desc' }) .then(() => cy.wrap(Cypress.vueWrapper.setProps({ fetcherCacheKey: '2' }))) // manually trigger refetch .get('@fetcher') - .should('have.callCount', 4) + .should('have.callCount', 5) .its('lastCall') .should('have.been.calledWith', { pageSize: 10, page: 2, offset: null, query: '', sortColumnKey: '', sortColumnOrder: 'desc' }) }) }) describe('misc', () => { - it('triggers the internal search and revalidate after clearing the search input', () => { + it('triggers the internal search and show cached results after clearing the search input', () => { const fns = { fetcher: ({ query }: { query: string }) => { return { data: [{ query }] } @@ -815,10 +817,9 @@ describe('KTableData', () => { .should('returned', { data: [{ query: 'some-keyword' }] }) .then(() => cy.wrap(Cypress.vueWrapper.setProps({ searchInput: '' }))) - // fetcher should be called immediately (< 350ms for search func) - cy.get('@fetcher', { timeout: 350 }) - .should('have.callCount', 3) // fetcher's 3rd call - .should('returned', { data: [{ query: '' }] }) + // fetcher should not be called as this state is already cached + cy.get('@fetcher', { timeout: 1000 }) + .should('have.callCount', 2) // fetcher's 3rd call }) }) }) From c7b59cc4e1589aae69123081dc78d26a53279a40 Mon Sep 17 00:00:00 2001 From: Justineo Date: Fri, 29 Nov 2024 17:02:58 +0800 Subject: [PATCH 4/6] chore(ktabledata): add debug log --- src/components/KTableData/KTableData.vue | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/components/KTableData/KTableData.vue b/src/components/KTableData/KTableData.vue index bd97137938..bf5afcdeba 100644 --- a/src/components/KTableData/KTableData.vue +++ b/src/components/KTableData/KTableData.vue @@ -554,6 +554,8 @@ watch([stateData, tableState], (newState) => { state: newTableState, hasData: newStateData.hasData, }) + + console.log(state, tableFetcherCacheKey.value) }) // handles debounce of search query From 42ea23db1d1be9e75a10e81ccd45e5c0243e4c76 Mon Sep 17 00:00:00 2001 From: Justineo Date: Tue, 3 Dec 2024 11:57:34 +0800 Subject: [PATCH 5/6] fix(ktabledata): use auto revalidation --- src/components/KTableData/KTableData.cy.ts | 10 +++----- src/components/KTableData/KTableData.vue | 29 ++-------------------- 2 files changed, 6 insertions(+), 33 deletions(-) diff --git a/src/components/KTableData/KTableData.cy.ts b/src/components/KTableData/KTableData.cy.ts index 0b68b715c7..c03b53ab4d 100644 --- a/src/components/KTableData/KTableData.cy.ts +++ b/src/components/KTableData/KTableData.cy.ts @@ -709,13 +709,12 @@ describe('KTableData', () => { cy.get('.table tbody').find('tr').should('have.length', 2) cy.get('.table tbody').should('contain.text', 'row10') cy.get('@fetcher') - // TODO: Investigate why fetcher is called twice here - .should('have.callCount', 4) + .should('have.callCount', 3) .its('lastCall') .should('have.been.calledWith', { pageSize: 10, page: 2, offset: '10', query: '', sortColumnKey: '', sortColumnOrder: 'desc' }) .then(() => cy.wrap(Cypress.vueWrapper.setProps({ fetcherCacheKey: '2' }))) // manually trigger refetch .get('@fetcher') - .should('have.callCount', 5) + .should('have.callCount', 4) .its('lastCall') .should('have.been.calledWith', { pageSize: 10, page: 2, offset: '10', query: '', sortColumnKey: '', sortColumnOrder: 'desc' }) }) @@ -768,13 +767,12 @@ describe('KTableData', () => { cy.get('.table tbody').find('tr').should('have.length', 2) cy.get('.table tbody').should('contain.text', 'row10') cy.get('@fetcher') - // TODO: Investigate why fetcher is called twice here - .should('have.callCount', 4) + .should('have.callCount', 3) .its('lastCall') .should('have.been.calledWith', { pageSize: 10, page: 2, offset: null, query: '', sortColumnKey: '', sortColumnOrder: 'desc' }) .then(() => cy.wrap(Cypress.vueWrapper.setProps({ fetcherCacheKey: '2' }))) // manually trigger refetch .get('@fetcher') - .should('have.callCount', 5) + .should('have.callCount', 4) .its('lastCall') .should('have.been.calledWith', { pageSize: 10, page: 2, offset: null, query: '', sortColumnKey: '', sortColumnOrder: 'desc' }) }) diff --git a/src/components/KTableData/KTableData.vue b/src/components/KTableData/KTableData.vue index bf5afcdeba..a5aa507f38 100644 --- a/src/components/KTableData/KTableData.vue +++ b/src/components/KTableData/KTableData.vue @@ -19,7 +19,7 @@ :hide-pagination="hidePagination || !showPagination" :hide-pagination-when-optional="false" :hide-toolbar="hideToolbar" - :loading="loading || isTableLoading || isRevalidating" + :loading="loading || isTableLoading" :max-height="maxHeight" :nested="nested" :pagination-attributes="tablePaginationAttributes" @@ -534,17 +534,7 @@ watch(fetcherData, (fetchedData: Record[]) => { // we want to tie loader to 'pending' since 'validating' is triggered even when pulling from cache, which should result in no loader // however, if this is a manual revalidation (triggered by page change, query, etc), display loader when validating watch(state, () => { - switch (state.value) { - case swrvState.PENDING: - isTableLoading.value = true - break - case swrvState.VALIDATING_HAS_DATA: - isTableLoading.value = isRevalidating.value - break - default: - isTableLoading.value = false - break - } + isTableLoading.value = state.value === swrvState.PENDING }, { immediate: true }) watch([stateData, tableState], (newState) => { @@ -571,7 +561,6 @@ watch(() => props.searchInput, (newSearchInput: string) => { } }, { immediate: true }) -const isRevalidating = ref(false) watch([query, page, pageSize], async (newData, oldData) => { const [oldQuery] = oldData const [newQuery, newPage] = newData @@ -581,20 +570,6 @@ watch([query, page, pageSize], async (newData, oldData) => { offsets.value = [null] offset.value = null } - - // don't revalidate until we have finished initializing and made initial fetch - if (hasInitialized.value && !isInitialFetch.value) { - isRevalidating.value = true - - if (newQuery !== '' && newQuery !== oldQuery) { - // handles debounce of search request - await debouncedRevalidate() - } else { - await revalidate() - } - - isRevalidating.value = false - } }, { deep: true, immediate: true }) onMounted(() => { From b146bb175c8fb5ca3895cf3041cc6949750c9cf0 Mon Sep 17 00:00:00 2001 From: Justineo Date: Tue, 3 Dec 2024 13:17:55 +0800 Subject: [PATCH 6/6] fix(ktabledata): simplify filter change --- src/components/KTableData/KTableData.cy.ts | 7 +++++-- src/components/KTableData/KTableData.vue | 18 ++++++------------ 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/components/KTableData/KTableData.cy.ts b/src/components/KTableData/KTableData.cy.ts index c03b53ab4d..db02c75598 100644 --- a/src/components/KTableData/KTableData.cy.ts +++ b/src/components/KTableData/KTableData.cy.ts @@ -810,14 +810,17 @@ describe('KTableData', () => { .then(() => cy.wrap(Cypress.vueWrapper.setProps({ searchInput: 'some-keyword' }))) // fetcher call should be delayed (> 350ms for search func + 500ms for revalidate func) + cy.get('@fetcher', { timeout: 200 }) + .should('have.callCount', 1) cy.get('@fetcher', { timeout: 1000 }) // fetcher's 2nd call .should('have.callCount', 2) // fetcher should be called once .should('returned', { data: [{ query: 'some-keyword' }] }) .then(() => cy.wrap(Cypress.vueWrapper.setProps({ searchInput: '' }))) // fetcher should not be called as this state is already cached - cy.get('@fetcher', { timeout: 1000 }) - .should('have.callCount', 2) // fetcher's 3rd call + cy.get('@fetcher', { timeout: 200 }) + .should('have.callCount', 3) // fetcher's 3rd call + .should('returned', { data: [{ query: '' }] }) }) }) }) diff --git a/src/components/KTableData/KTableData.vue b/src/components/KTableData/KTableData.vue index a5aa507f38..7797f13a9e 100644 --- a/src/components/KTableData/KTableData.vue +++ b/src/components/KTableData/KTableData.vue @@ -304,7 +304,7 @@ const getCellSlots = computed((): string[] => { const fetcherParams = computed(() => ({ pageSize: pageSize.value, page: page.value, - query: props.searchInput || filterQuery.value, + query: filterQuery.value, sortColumnKey: sortColumnKey.value, sortColumnOrder: sortColumnOrder.value, offset: offset.value, @@ -399,9 +399,8 @@ const tableFetcherCacheKey = computed((): string => { return `k-table_${identifierKey}` }) -const query = ref('') const { debouncedFn: debouncedSearch, generateDebouncedFn: generateDebouncedSearch } = useDebounce((q: string) => { - query.value = q + filterQuery.value = q }, 350) const search = generateDebouncedSearch(0) // generate a debounced function with zero delay (immediate) @@ -420,8 +419,7 @@ const stateData = computed((): SwrvStateData => ({ state: state.value as SwrvState, })) const tableState = computed((): TableState => isTableLoading.value ? 'loading' : fetcherError.value ? 'error' : 'success') -const { debouncedFn: debouncedRevalidate, generateDebouncedFn: generateDebouncedRevalidate } = useDebounce(_revalidate, 500) -const revalidate = generateDebouncedRevalidate(0) // generate a debounced function with zero delay (immediate) +const { debouncedFn: debouncedRevalidate } = useDebounce(_revalidate, 500) const sortHandler = ({ sortColumnKey: columnKey, prevKey, sortColumnOrder: sortOrder }: TableSortPayload): void => { const header: TableDataHeader = tableHeaders.value.find((header) => header.key === columnKey)! @@ -550,10 +548,6 @@ watch([stateData, tableState], (newState) => { // handles debounce of search query watch(() => props.searchInput, (newSearchInput: string) => { - if (page.value !== 1) { - page.value = 1 - } - if (newSearchInput === '') { search(newSearchInput) } else { @@ -561,11 +555,11 @@ watch(() => props.searchInput, (newSearchInput: string) => { } }, { immediate: true }) -watch([query, page, pageSize], async (newData, oldData) => { +watch([filterQuery, pageSize], async (newData, oldData) => { const [oldQuery] = oldData - const [newQuery, newPage] = newData + const [newQuery] = newData - if (newQuery !== oldQuery && newPage !== 1) { + if (newQuery !== oldQuery && page.value !== 1) { page.value = 1 offsets.value = [null] offset.value = null