Standardize single doc deployment #698
Conversation
R/deployApp.R
Outdated
| if (dirExists(appDir)) { | ||
| # ok | ||
| } else if (file.exists(appDir) && isStaticFile(appDir)) { | ||
| doc <- standardizeSingleDocDeployment(appDir) |
There was a problem hiding this comment.
One thing that's slightly unappealing about this approach is that we use a different approach to determining appFiles depending on whether you do deployApp("foo.Rmd") or deployApp(appPrimaryDoc = "foo.Rmd"). An alternative approach would be to add an appPrimaryDoc argument to appStandardizeFiles() and call rmarkdown::find_external_dependencies() there when appPrimaryDoc is supplied.
There was a problem hiding this comment.
Another approach, since this never appears to have been documented, would be to deprecate this use appDir and tell folks to call deployDoc() instead.
There was a problem hiding this comment.
Interesting. I forgot about this difference.
# Uses the default appDir=getwd() and enumerates all files in the directory for bundling
deployApp(appPrimaryDoc = "foo.Rmd")
# Uses the foo.Rmd to identify the dependencies of the document; only those are bundled
deployDoc("foo.Rmd")The existing deployApp("foo.Rmd") feels like a convenience, but we should nudge folks towards using deployDoc should they provide a file.
Added in: #16, and before the notion of a "primary doc".
I'm uneasy about the automatic use of rmarkdown::find_external_dependencies. On the one hand, it does feel like it will produce the set of files that most folks want to include -- directory explosion is often too greedy. On the other hand, its use would add to the "magic" that happens in the "lower-level" deployApp. Its inclusion could suddenly force more folks to compute their own appFiles.
There was a problem hiding this comment.
Deprecating the deployApp -> deployDoc -> deployApp workflow feels like the right path.
There was a problem hiding this comment.
Yeah, I think the fundamentally different mechanism of file discovery suggests that this is a deployDoc() feature, and we should deprecate this old path.
R/deployApp.R
Outdated
| if (dirExists(appDir)) { | ||
| # ok | ||
| } else if (file.exists(appDir) && isStaticFile(appDir)) { | ||
| doc <- standardizeSingleDocDeployment(appDir) |
There was a problem hiding this comment.
Interesting. I forgot about this difference.
# Uses the default appDir=getwd() and enumerates all files in the directory for bundling
deployApp(appPrimaryDoc = "foo.Rmd")
# Uses the foo.Rmd to identify the dependencies of the document; only those are bundled
deployDoc("foo.Rmd")The existing deployApp("foo.Rmd") feels like a convenience, but we should nudge folks towards using deployDoc should they provide a file.
Added in: #16, and before the notion of a "primary doc".
I'm uneasy about the automatic use of rmarkdown::find_external_dependencies. On the one hand, it does feel like it will produce the set of files that most folks want to include -- directory explosion is often too greedy. On the other hand, its use would add to the "magic" that happens in the "lower-level" deployApp. Its inclusion could suddenly force more folks to compute their own appFiles.
R/deployApp.R
Outdated
| if (dirExists(appDir)) { | ||
| # ok | ||
| } else if (file.exists(appDir) && isStaticFile(appDir)) { | ||
| doc <- standardizeSingleDocDeployment(appDir) |
There was a problem hiding this comment.
Deprecating the deployApp -> deployDoc -> deployApp workflow feels like the right path.
Extract out
standardizeSingleDocDeployment()and use it indeployDoc()anddeployApp().I've also documented this feature of
deployApp(), which lead to some other minor doc changes since other function inherited fromdeployApp(). This does raise a question of whether these functions should also support anappDirthat's actually a path to a document.