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

Possible loop on datatable with server-side interaction #1834

Closed
danielo515 opened this issue Mar 24, 2018 · 5 comments
Closed

Possible loop on datatable with server-side interaction #1834

danielo515 opened this issue Mar 24, 2018 · 5 comments
Milestone

Comments

@danielo515
Copy link

Software version

Quasar: 0.15.2
OS: MacOS X
Node: 8.10
NPM: 5
Browsers: Chrome

JsFiddle (for Quasar v0.15+ only)

What did you get as the error?

I don't get any error, the problem is the behavior

What were you expecting?

I expect to not trigger the server-side pagination more than once per server interaction.

What steps did you take, to get the error?

I'm going to explain the entire problem here.

This is related to datatable and server-side pagination/filtering. Reading the following two code snippets related to pagination it becomes clear what the problem is:

This is how pages number is calculated

pagesNumber () {
const { rowsPerPage } = this.computedPagination
return Math.ceil(this.computedRowsNumber / rowsPerPage)
},

This the watcher of pages number and the set pagination method

watch: {
pagesNumber (lastPage) {
const currentPage = this.computedPagination.page
if (lastPage && !currentPage) {
this.setPagination({ page: 1 })
}
else if (lastPage < currentPage) {
this.setPagination({ page: lastPage })
}
}
},
methods: {
setPagination (val) {
const newPagination = extend({}, this.computedPagination, val)
if (this.isServerSide) {
this.requestServerInteraction({
pagination: newPagination
})
return
}
if (this.pagination) {
this.$emit('update:pagination', newPagination)
}
else {
this.innerPagination = newPagination
}
},

Now think about what will happen if the server returns an empty list of results. Rows number will be 0, so pagesNumber will return 0. Because of this, pagesNumber watcher will evaluate the second if to true (because last page is 0, and currentPage is 1) and will call setPagination, which in turn will call requestServerInteraction (with page set to 0!) which will call the server again, who will return an empty results list again (2 server trips for now).
If you just trust the reported page (0) and saves it depending on your situation this may be a problem: if your pagination is 0 based, and you subtract 1 from the reported page (because datatable is 1 based) you will end requesting the -1 page to the server.
At this point because the watcher looks for changes the loop may end here, but if you modify the page in any way to keep it at 1 for example, it will be different from the last page (0) and it will trigger again.

I'm not sure how to fix this on quasar itself. Maybe it is not a good idea to allow pagesNumber to become 0, and it should be set to a minimum of 1.

Regards

@rstoenescu
Copy link
Member

Fix available in 0.15.11

@Lou-Der
Copy link
Contributor

Lou-Der commented May 11, 2018

Hi,

I've been testing the dev branch and the new fixPagination code you added :

if (p.rowsPerPage !== void 0 && p.rowsPerPage < 1) {
    p.rowsPerPage = 5
  }

Causes the "All" rowsPerPage to be set back to 5 rows per page instead. Option All = 0 as you know...

@danielo515
Copy link
Author

To be honest, using 0 which has a meaning of, well zero, to flag all seems like a bad design decission

@rstoenescu
Copy link
Member

@Lou-Der will investigate. thanks for reporting.
@danielo515 yes, because displaying 0 rows makes so much sense, right? :)))

@danielo515
Copy link
Author

True, I was thinking about page zero, not zero rows

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

3 participants