search

rstudio / bslib

Tools for theming Shiny and R Markdown via Bootstrap 3, 4, or 5.
https://rstudio.github.io/bslib/
Other
469 stars 57 forks source link

Upgrade `bs_theme(version=4)` to 4.6.1 #422

Open tillea opened 2 years ago

tillea commented 2 years ago

Hi, as mentioned in issue #412 we try to package bslib for Debian (as a predependency for other packages like shiny). I've just mentioned that we need to provide the sources for compressed JS files. Alternatively we can link to just packaged code inside Debian which works nicely for bootstrap3 since its the very same version as in bslib. Unfortunately for bootstrap4 we have a minor discrepancy in the versioning. Debian has 4.6.1 while bslib ships 4.6.0. It turns out that the file _deprecated.scss was droped in version 4.6.1 which leads to the following test suite errors when I'm replacing bootstrap4 shipped with bslib by the Debian packaged version:

Error in `sass_file(f)`: Could not find file: '/usr/lib/R/site-library/bslib/lib/bs4/scss/./_deprecated.scss' in dir: /tmp/autopkgtest-lxc.re9ddi7e/downtmp/autopkgtest_tmp/tests/testthat

There is a full build log in our CI instance which shows 27 of these failures. I admit I have no idea why the code is exactly checking for that missing file. However, I wonder whether it would be the easiest solution if you would simply switch bslib to that bugfix release of bootstrap4. This would leave me only the task to deal with inst/lib/bs5/dist/js/bootstrap.bundle.min.js* (where I need to provide the uncompressed source to be acceptable for Debian distribution).

Kind regards, Andreas.

cpsievert commented 2 years ago

when I'm replacing bootstrap4 shipped with bslib by the Debian packaged version:

Sorry, but come of think of it, even if we do upgrade, it won't change the fact that we apply patches to the SCSS source code for BS4 and BS5 (to fix bugs and add features). Technically you'll probably be fine replacing inst/lib/bs3 with whatever Debian bundles, but in generally this seems like a dangerous practice that could lead to surprising and difficult to debug regressions for end users (I hope you're not planning on doing the same for other HTML dependencies in shiny as we also applies patches to source there as well)

tillea commented 2 years ago

Am Thu, Apr 14, 2022 at 07:51:05AM -0700 schrieb Carson Sievert:

Sorry, but come of think of it, even if we do upgrade, it won't change the fact that we apply patches to the SCSS source code for BS4 and BS5 (to fix bugs and add features). Technically you'll probably be fine replacing inst/lib/bs3 with whatever Debian bundles, but in generally this seems like a dangerous practice that could lead to surprising and difficult to debug regressions for end users (I hope you're not planning on doing the same for other HTML dependencies in shiny as we also applies patches to source there as well)

Thanks a lot for this hint. Do you see any chance to distribute the uncompressed source of the JS files? This would also help a lot. Kind regards, Andreas.

cpsievert commented 2 years ago

Once we upgrade to 4.6.1 I think that should be OK, but I can't guarantee we won't make more patches in the future the JS, so you might want to test for that in your build system (i.e., throw a failure if our inst/lib files don't match what's available on Debian)