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

Add sustainEnvAndQuoted(). Remove getQuosure() #3468

Merged
merged 22 commits into from
Jul 26, 2021

Conversation

schloerke
Copy link
Collaborator

@schloerke schloerke commented Jul 23, 2021

Related: #3462
Related: #3466

exprToFunction() (and installExprFunction()) do not work in all situations and reaching up into the calling code is dangerous / should be avoided. Now, the dangerous code is isolated to sustainEnvAndQuoted(), which ideally package authors will stop using.

With the updated code, all handling will be done with quosures. woot.


Shiny 1.6.0 code

renderDouble <- function(expr, env = parent.frame(), quoted = FALSE) {
	func <- exprToFunction(expr, env, quoted)
	function() {
		rep(func(), 2)
	}
}

Shiny 1.7.0 migration code

renderDouble <- function(expr, env = deprecated(), quoted = deprecated()) {
	q <- enquo0(expr)
	q <- sustainEnvAndQuoted(q, expr, env, quoted)
    func <- quoToFunction(q)
	function() {
		rep(func(), 2)
	}
}

Ideal Shiny 1.7.0 code

renderDouble <- function(expr) {
    func <- quoToFunction(enquo0(expr))
	function() {
		rep(func(), 2)
	}
}

Other changes

  • Allow shinyDeprected() to know of superseded functions
  • Document
    • Add superseded badge to renderDataTable()
    • Add superseded badge to exprToFunction(), installExprFunction()

Remaining:

  • Document TODO deprecated() parameters
  • Document upgrade/migration path for shinyRenderWidget
  • Document quoToFunction() & handleEnvAndQuoted()
    • Explain why we're moving to quosures. One reason is that we don't have to reach to different levels using substitute. Once you have a quosure, it can be passed around safely. Normal NSE expressions are like bombs; quosures are safe to pass around.

R/reactives.R Outdated Show resolved Hide resolved
R/reactives.R Outdated Show resolved Hide resolved
R/utils-lang.R Show resolved Hide resolved
@schloerke schloerke changed the title Allow for shinyDeprecated() to know of _superseded_ functions Add handleEnvAndQuoted(). Remove getQuosure() Jul 26, 2021
R/utils-lang.R Outdated Show resolved Hide resolved
@wch
Copy link
Collaborator

wch commented Jul 26, 2021

We can update the docs in a follow-up PR.

@schloerke schloerke changed the title Add handleEnvAndQuoted(). Remove getQuosure() Add sustainEnvAndQuoted(). Remove getQuosure() Jul 26, 2021
@schloerke schloerke marked this pull request as ready for review July 26, 2021 21:53
@schloerke schloerke merged commit 8b74338 into master Jul 26, 2021
@schloerke schloerke deleted the barret/quosure_expr_to_func branch July 26, 2021 21:54
schloerke added a commit that referenced this pull request Aug 3, 2021
* master:
  Update `esbuild-plugin-sass` to latest version (#3463)
  Interpret NULL discrete limits as NA, fixes #2666 (#2668)
  Add `sustainEnvAndQuoted()`. Remove `getQuosure()` (#3468)
  Update NEWS
  Fix handling of getQuosure3(expr, env, quoted=TRUE)
  Fix NEWS entry
  Update NEWS
  Fix example
  Update pkgdown.yml
  Update documentation
  Add quosure tests for custom render functions
  Update comment
  Export getQuosure() and add internal getQuosure3()
  Rename get_quosure to getQuosure
  Modify exprToFunction to accept quosures
  Move expression/quosure functions to utils-lang.R
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants