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

fix(ktabledata): fetcher cache key should respect fetcher params #2531

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/components/KTableData/KTableData.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,7 @@ describe('KTableData', () => {
})

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 }] }
Expand Down Expand Up @@ -810,13 +810,15 @@ 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 be called immediately (< 350ms for search func)
cy.get('@fetcher', { timeout: 350 })
// fetcher should not be called as this state is already cached
cy.get('@fetcher', { timeout: 200 })
.should('have.callCount', 3) // fetcher's 3rd call
.should('returned', { data: [{ query: '' }] })
})
Expand Down
68 changes: 21 additions & 47 deletions src/components/KTableData/KTableData.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -301,17 +301,18 @@ 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: filterQuery.value,
sortColumnKey: sortColumnKey.value,
sortColumnOrder: sortColumnOrder.value,
offset: offset.value,
}))

const isInitialFetch = ref<boolean>(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<string, any>[]
total.value = props.paginationAttributes?.totalCount || res.total || res.data?.length || 0

Expand Down Expand Up @@ -389,16 +390,17 @@ const tableFetcherCacheKey = computed((): string => {
identifierKey = props.cacheIdentifier
}

identifierKey += `-${JSON.stringify(fetcherParams.value)}`

if (props.fetcherCacheKey) {
identifierKey += `-${props.fetcherCacheKey}`
}

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)

Expand All @@ -417,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)!
Expand Down Expand Up @@ -531,17 +532,7 @@ watch(fetcherData, (fetchedData: Record<string, any>[]) => {
// 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) => {
Expand All @@ -551,45 +542,28 @@ watch([stateData, tableState], (newState) => {
state: newTableState,
hasData: newStateData.hasData,
})

console.log(state, tableFetcherCacheKey.value)
})

// handles debounce of search query
watch(() => props.searchInput, (newSearchInput: string) => {
if (page.value !== 1) {
page.value = 1
}

if (newSearchInput === '') {
search(newSearchInput)
} else {
debouncedSearch(newSearchInput)
}
}, { immediate: true })

const isRevalidating = ref<boolean>(false)
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
}

// 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(() => {
Expand Down
Loading