- Notifications
You must be signed in to change notification settings - Fork85
Sketch dryRun() implementation#833
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
hadley commentedMay 8, 2023
@aronatkins@kevinushey I still need to think up a testing strategy, but I think the overall strategy is feeling pretty solid and it'd be useful to get your thoughts. |
| #' @description | ||
| #' `r lifecycle::badge("experimental")` | ||
| #' | ||
| #' `dryRun()` runs your app locally, attempting to simulate what will happen |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
nit: s/app/content/
| appRunner <- function(appMode) { | ||
| switch(appMode, | ||
| "rmd-static" = , | ||
| "rmd-shiny" = function(primaryDoc) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
rmd-shiny should usermarkdown::run(file = NULL, dir = getwd())
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
When the R Markdown content is a site:rmarkdown::render_site(envir = new.env())
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I already set the working directory above, but I'll take the other changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The::run(..) signature is necessary to have R Markdown do its "run the whole directory" magic.. it's not setting the working directory. I think that's this logic:https://github.com/rstudio/rmarkdown/blob/main/R/shiny.R#L83-L107
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I just meant thedir argument
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
OK. Once you givefile = NULL, thedir value is necessary, as its default value ofdirname(file) is meaningless.
| rmarkdown::render(primaryDoc, quiet = TRUE) | ||
| }, | ||
| "quarto-static" = , | ||
| "quarto-shiny" = function(primaryDoc) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
quarto-shiny should:
- render with Quarto
- use
rmarkdown::run(file = NULL, dir = getwd())
| "api" = function(primaryDoc) { | ||
| plumber::pr_run(plumber::pr("plumber.R")) | ||
| }, | ||
| cli::cli_abort("Content type {appMode} not currently supported") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Rendered content and "static" -- we could start an HTML server against thebundleDir so they could see exactly what files they are about to deploy. Maybe that would help cases where folks forget to include some images/CSS.
The result of this would be thatevery content type starts some type of HTTP server that they would interact with for validation.
| #' Both of these will work locally but fail on the server. [lint()] | ||
| #' uses an alternative technique (static analysis) to detect many of these | ||
| #' cases. | ||
| #' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Another limitation is that:
dryRun()doesn't help if the server does not have required system dependencies that are present
on the machine on which you're running this dryRun().
Maybe a bit nuanced to mention?
Uh oh!
There was an error while loading.Please reload this page.
Tofix#725.
Fixes#208.Fixes#253.Fixes#256.