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: Only copy pre-compiled themes once #1184

Merged
merged 9 commits into from
Feb 18, 2025
153 changes: 88 additions & 65 deletions R/bs-dependencies.R
Original file line number Diff line number Diff line change
Expand Up @@ -71,71 +71,10 @@ bs_theme_dependencies <- function(
cache <- sass_cache_get_dir(cache)
}

out_file <- NULL
# Look for a precompiled css file if user asks for it AND the default options
# are used.
if (
precompiled &&
identical(sass_options, sass_options(output_style = "compressed"))
) {
precompiled_css <- precompiled_css_path(theme)
if (!is.null(precompiled_css)) {
out_dir <- file.path(tempdir(), paste0("bslib-precompiled-", version))
if (!dir.exists(out_dir)) {
dir.create(out_dir)
}
out_file <- file.path(out_dir, basename(precompiled_css))
file.copy(precompiled_css, out_file)

# Usually sass() would handle file_attachments and dependencies,
# but we need to do this manually
out_file <- attachDependencies(out_file, htmlDependencies(as_sass(theme)))
write_file_attachments(
as_sass_layer(theme)$file_attachments,
out_dir
)
}
}
out_file <- maybe_precompiled_css(theme, sass_options, precompiled)

# If precompiled css not found, compile normally.
if (is.null(out_file)) {
contrast_warn <- get_shiny_devmode_option(
"bslib.color_contrast_warnings",
default = FALSE,
devmode_default = TRUE,
devmode_message = paste(
"Enabling warnings about low color contrasts found inside `bslib::bs_theme()`.",
"To suppress these warnings, set `options(bslib.color_contrast_warnings = FALSE)`"
)
)
theme <- bs_add_variables(theme, "color-contrast-warnings" = contrast_warn)

out_file <- sass(
input = theme,
options = sass_options,
output = output_template(basename = "bootstrap", dirname = "bslib-"),
cache = cache,
write_attachments = TRUE,
cache_key_extra = list(
get_exact_version(version),
get_package_version("bslib")
)
)
}

out_file_dir <- dirname(out_file)

js_files <- bootstrap_javascript(version)
js_map_files <- bootstrap_javascript_map(version)
success_js_files <- file.copy(
c(js_files, js_map_files),
out_file_dir,
overwrite = TRUE
)
if (any(!success_js_files)) {
warning(
"Failed to copy over bootstrap's javascript files into the htmlDependency() directory."
)
out_file <- sass_compile_theme(theme, sass_options, cache)
}

htmltools::resolveDependencies(
Expand All @@ -145,9 +84,9 @@ bs_theme_dependencies <- function(
htmlDependency(
name = "bootstrap",
version = get_exact_version(version),
src = out_file_dir,
src = dirname(out_file),
stylesheet = basename(out_file),
script = basename(js_files),
script = basename(bootstrap_javascript(version)),
all_files = TRUE, # include font and map files
meta = list(
viewport = "width=device-width, initial-scale=1, shrink-to-fit=no"
Expand All @@ -159,6 +98,90 @@ bs_theme_dependencies <- function(
)
}

maybe_precompiled_css <- function(theme, sass_options, precompiled) {
if (!precompiled) return()
if (!identical(sass_options, sass_options(output_style = "compressed"))) {
return()
}

precompiled_css <- precompiled_css_path(theme)
if (is.null(precompiled_css)) {
return()
}

version <- theme_version(theme)
deps_hash <- rlang::hash(htmlDependencies(as_sass(theme)))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably over-thinking things. The one way that the theme could change between calls is if the Bootstrap JavaScript assets change. Taking the hash of the dependencies was intended to catch this, but we'd actually have to hash the files themselves. Not sure if it's worth it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not hard, maybe still not worth it: 58f62cd

out_dir <- file.path(
tempdir(),
sprintf("bslib-precompiled-%s-%s", version, deps_hash)
)

out_file <- file.path(out_dir, basename(precompiled_css))
out_file <- attachDependencies(out_file, htmlDependencies(as_sass(theme)))

if (dir.exists(out_dir)) {
# We've already copied all the files for this precompiled theme, there's no
# need to copy them again.
return(out_file)
}

dir.create(out_dir)
file.copy(precompiled_css, out_file)

# Usually sass() would handle file_attachments and dependencies,
# but we need to do this manually
write_file_attachments(
as_sass_layer(theme)$file_attachments,
out_dir
)

bootstrap_javascript_copy_assets(version, dirname(out_file))

out_file
}

sass_compile_theme <- function(theme, sass_options, sass_cache) {
version <- theme_version(theme)

contrast_warn <- get_shiny_devmode_option(
"bslib.color_contrast_warnings",
default = FALSE,
devmode_default = TRUE,
devmode_message = paste(
"Enabling warnings about low color contrasts found inside `bslib::bs_theme()`.",
"To suppress these warnings, set `options(bslib.color_contrast_warnings = FALSE)`"
)
)
theme <- bs_add_variables(theme, "color-contrast-warnings" = contrast_warn)

out_file <- sass(
input = theme,
options = sass_options,
output = output_template(basename = "bootstrap", dirname = "bslib-"),
cache = sass_cache,
write_attachments = TRUE,
cache_key_extra = list(
get_exact_version(version),
get_package_version("bslib")
)
)

bootstrap_javascript_copy_assets(version, dirname(out_file))

out_file
}

bootstrap_javascript_copy_assets <- function(version, to) {
js_files <- bootstrap_javascript(version)
js_map_files <- bootstrap_javascript_map(version)
success_js_files <- file.copy(c(js_files, js_map_files), to, overwrite = TRUE)
if (any(!success_js_files)) {
warning(
"Failed to copy over bootstrap's javascript files into the htmlDependency() directory."
)
}
}

#' Themeable HTML components
#'
#' @description
Expand Down
Binary file modified R/sysdata.rda
Binary file not shown.
2 changes: 1 addition & 1 deletion inst/components/dist/components.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading