rstudio / DT

R Interface to the jQuery Plug-in DataTables
https://rstudio.github.io/DT/
Other
587 stars 184 forks source link

Column filters with column class `fs::bytes` cause subsequent datatables to disappear #993

Open thomascwells opened 2 years ago

thomascwells commented 2 years ago

If I create a DT::datatable() using a data.frame that has a column with class fs::bytes (such as those generated by fs::dir_info()), everything works as expected. But if I additionally add column filters to that datatable, it causes any other datatables further down in the document to not display.

I can reproduce the error rendering to both html_document and flexdashboard.

Here is a minimal (I hope!) reproducible example:

---
title: "DT filter bug when using fs::bytes column"
output: html_document
---

```{r}
library('fs')
library('dplyr')
library('DT')

Working trees DT, with filter

trees %>% 
  glimpse() %>% 
  datatable(filter = 'top')

Working DT with fs::bytes column, no filter

fs_table <- data.frame(numeric_bytes = c(1, 2e9, 3e9, 4e12)) %>%
  mutate(fs_bytes_col = fs::fs_bytes(numeric_bytes)) %>%
  glimpse()

fs_table %>% datatable()

Working trees DT - demonstrates nothing is broken yet.

trees %>% datatable()

DT with fs::bytes column, with filter - This works, but I think this table creates the issue

fs_table %>% datatable(filter = 'top')

Broken examples - There should be tables here, like above, but nothing is shown.

trees %>% datatable()
trees %>% datatable(filter = 'top')
fs_table %>% datatable()

Screenshot of the bottom of the rendered html_document: 

![image](https://user-images.githubusercontent.com/15213768/163288789-d6b675dc-2263-4d18-90a2-474d4693d2eb.png)

Session info: 

xfun::session_info('DT') R version 4.1.3 (2022-03-10) Platform: x86_64-w64-mingw32/x64 (64-bit) Running under: Windows 10 x64 (build 22000), RStudio 2022.2.0.443

Locale: LC_COLLATE=English_United States.1252 LC_CTYPE=English_United States.1252
LC_MONETARY=English_United States.1252 LC_NUMERIC=C
LC_TIME=English_United States.1252

Package version: base64enc_0.1.3 crosstalk_1.2.0 digest_0.6.29 DT_0.22.1 fastmap_1.1.0
graphics_4.1.3 grDevices_4.1.3 htmltools_0.5.2 htmlwidgets_1.5.4 jquerylib_0.1.4
jsonlite_1.8.0 later_1.3.0 lazyeval_0.2.2 magrittr_2.0.3 methods_4.1.3
promises_1.2.0.1 R6_2.5.1 Rcpp_1.0.8.3 rlang_1.0.2 stats_4.1.3
utils_4.1.3 yaml_2.3.5



Happy to help if there's anything else I can do! 

---

By filing an issue to this repo, I promise that

- [X] I have fully read the issue guide at https://yihui.name/issue/.
- [X] I have provided the necessary information about my issue.
    - If I'm asking a question, I have already asked it on Stack Overflow or RStudio Community, waited for at least 24 hours, and included a link to my question there.
    - If I'm filing a bug report, I have included a minimal, self-contained, and reproducible example, and have also included `xfun::session_info('DT')`. I have upgraded all my packages to their latest versions (e.g., R, RStudio, and R packages), and also tried the development version: `remotes::install_github('rstudio/DT')`.
    - If I have posted the same issue elsewhere, I have also mentioned it in this issue.
- [X] I have learned the Github Markdown syntax, and formatted my issue correctly.

I understand that my issue may be closed if I don't fulfill my promises.
wholmes105 commented 2 years ago

Are you sure the table with the filters isn't also broken? When I try to click on the second column filter for fs_table, I get a broken dropdown, and the error in my web browser's console when the page is loaded suggests the added fs_bytes class of fs_table$fs_bytes_col is interfering with the JavaScript side of things partway through when the filter is being created.

You could work around this by using as.numeric(fs_table$fs_bytes_col) to remove the fs_bytes class from fs_table$fs_bytes_col, and just apply that process to every column in your data.frame as needed.

thomascwells commented 2 years ago

Are you sure the table with the filters isn't also broken?

Interesting! Yes, I get the same broken dropdown in that table. There is certainly something wrong with that table, but I think it's especially odd (concerning?) that an error in one table would cause other tables to not render.

The as.numeric() workaround you suggested is what I landed on for now, and honestly it's a perfectly fine workaround for my purposes. I wanted to report the issue in case there might be a bigger bug lurking under the surface here.

I don't know enough about html or JavaScript to further debugging, but if it seems like this issue likely stems from the print methods (or something similar) of the fs_bytes class, I'm happy to bring it up in the fs repo.

I took a look at issues in fs mentioning fs_bytes but couldn't find anything that seems related. https://github.com/r-lib/fs/issues?q=is%3Aissue+fs_bytes

wholmes105 commented 2 years ago

I agree that each table should be independent of the others, but the print method in R doesn't have anything to do with it, I think. The DT package is an API to datatables.js, and it is the [draw](https://datatables.net/reference/api/draw()) method that is responsible for actually showing changes to the user; that being said, the column filters are added by DT and aren't part of datatables.js.

Digging around, it seems most likely that the error that appears whenever the column filters are created is what prevents subsequent filters in the same table from rendering properly, as well as subsequent tables from rendering at all. It is worth noting, however, that the <div> to hold the table still appears in the html of your document.

wholmes105 commented 2 years ago

I think the issue might have something to do with the for loop here that iterates through each column and creates the filters. Adding a try catch to the loop might prevent the issue from affecting other columns and tables, but I doubt it would fix the underlying problem. It seems a bit odd that this doesn't affect the table itself, though, so perhaps data for the column filters is treated differently than data for the table itself? Stripping extra classes from the data before importing to JavaScript might resolve this, but that seems like it could adversely affect something else.

diraol commented 1 year ago

👋🏽 Just passing by to say that I have a similar behavior while working in a Markdown Xaringan presentation (multiple slides) in which almost all slides have tables and I want to use the filter = 'top' in all of them.

If I enable the filter = 'top' feature in two tables the second one is not rendered at all. If I enable it in all tables, just the first one is rendered (and sometimes by moving back and forth the table get rendered (eventually).

So I guess it may not be exactly a problem with fs_bytes.

mikmart commented 5 months ago

This seems to happen because the calculated filter range for the fs_bytes column also retains the fs_bytes class. The range gets transmitted to the JavaScript side as character values in the DOM. But because converting fs_bytes to character returns the formatted version, the range slider creation gets character values rather than numeric values, causing the error.

as.character(fs::fs_bytes(5000))
#> [1] "4.88K"

The same problem will affect any numeric vector based classes that return a formatted value from as.character().

A potential fix could be to strip classes from the calculated slider range, if it's not a date or datetime.