Repository navigation
Change default behaviour for R package generator to recycle package version info #870
Description
Currently, the R package generation code inserts an integer (which increments for each additional asset inserted into the dependency list) for the version number when none is available in __init__.py.
It would be safer to default to inserting the same version information as for the containing package, since this currently requires developer intervention and editing of internal.R after the package is generated. A (fictional) example:
.dashAwesomeExtensions_js_metadata <- function() {
deps_metadata <- list(`dash_awesome_extensions` = structure(list(name = "dash_awesome_extensions",
version = "2.0", src = list(href = NULL,
file = "deps"), meta = NULL,
script = 'dash_awesome_extensions.min.js',
stylesheet = NULL, head = NULL, attachment = NULL, package = "dashAwesomeExtensions",
all_files = FALSE), class = "html_dependency"),
`dash_awesome_extensions` = structure(list(name = "dashAwesomeExtensions",
version = "1", src = list(href = NULL,
file = "deps"), meta = NULL,
script = NULL,
stylesheet = 'fancify.css', head = NULL, attachment = NULL, package = "dashDesignKit",
all_files = FALSE), class = "html_dependency"),
`dash_awesome_extensions` = structure(list(name = "dashAwesomeExtensions",
version = "2", src = list(href = NULL,
file = "deps"), meta = NULL,
script = NULL,
stylesheet = 'stylize.css', head = NULL, attachment = NULL, package = "dashDesignKit",
all_files = FALSE), class = "html_dependency"))
return(deps_metadata)
}
This is important because of the way Dash dependencies are currently fetched via URL:
https://github.com/plotly/dashR/blob/a44050a27ba55056afed8ebac81e852ddf852e1f/R/utils.R#L196-L204
I propose that the default should be to replace version = "1" and version = "2" with version = "2.0" in the example above if version information for dependencies is not available within a generated package.
The relevant code block is here:
dash/dash/development/_r_components_generation.py
Lines 278 to 286 in d9ddc87
Activity
alexcjohnson commentedon Aug 15, 2019
Let's take a step back here - AFAICT this version is just used in the cache-busting query param when we generate a
<script>or<link>tag. Right now we include both version and file mod time in this query param?v={}&m={}. We've had the version for a long time, mod time was added last fall in #387 - along withcomponents_cache_max_agewhich we've since removed as unnecessary specifically because we have mod time in all the urls.Is there really any reason we need both of these? Once we have mod time - which supports changes during development as well as users upgrading versions - isn't the version redundant? I propose we simply get rid of version and only use mod time.
rpkyle commentedon Aug 15, 2019
From my point of view, I think 🔪 the
v=part of the URL makes a lot of sense.I've yet to run into any generated packages/libraries which contain assets where (a) the asset version ≠ the package version, and (b) it's important to maintain this distinct version information.
rpkyle commentedon Aug 15, 2019
Speaking of which, can I introduce a simple PR to 🔪
components_cache_max_age, targeted atdev?alexcjohnson commentedon Aug 15, 2019
In
dashRyou mean? Absolutely!rpkyle commentedon Jan 7, 2020
🎉 This issue was resolved in #1048. Closing!