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

lines, parsed, source #329

Closed
schloerke opened this issue Nov 2, 2018 · 3 comments
Closed

lines, parsed, source #329

schloerke opened this issue Nov 2, 2018 · 3 comments
Labels
type: question Poses a question, not a problem

Comments

@schloerke
Copy link
Collaborator

@trestletech

Looking here: https://github.com/trestletech/plumber/blob/4e99e44532c958b84af642689ba64215c4624a12/R/plumber.R#L180-L183

My current understanding of the three components are as follows:

  • private$lines - used to parse the roxygen like tags for the function
  • private$parsed - used to find out which functions exists, their source lines, and using private$lines, it can parse a block for plumber
  • source(file, local = private$envir) ...
    • What is this used for? Populate the envir with globals in file?

I'm trying to make sure that my changes made in #328 are going to be correct and will have the same effect. I am moving away from source(file) and going towards eval(parse(file)) due to lack of useful utf8 support in source().

@schloerke schloerke added the type: question Poses a question, not a problem label Nov 2, 2018
@trestletech
Copy link
Contributor

Howdy. I think this is explained here https://www.rplumber.io/docs/runtime.html#environments

Basically, the goal is just to avoid muddying up the global environment when you plumb an API. And maybe this helps with reproducibility of APIs since they're less likely to pull in data from the global env accidentally?

Let me know if that doesn't sound right and I can take another look.

@schloerke
Copy link
Collaborator Author

Ok! I believe we're narrowing it down then.

I believe the code is being sourced twice then. Once with the source call and the second with the eval call inside activate_block when the endpoint is created.

Source: https://github.com/trestletech/plumber/blob/4e99e44532c958b84af642689ba64215c4624a12/R/plumber.R#L183

Endpoint: https://github.com/trestletech/plumber/blob/4e99e44532c958b84af642689ba64215c4624a12/R/plumber-step.R#L117


The file below makes a function that returns valueA, which is defined afterwards.

#* @get /
(function(){
  cat("sourced!\n")
  function() {
    valueA
  }
})()

valueA <- 4

This currently works because the original whole file sourcing populates the local environment with local variables.

If the original whole file sourcing is removed, the above example fails with an unknown valueA as valueA is not defined at expression eval time.


Ok. I'm going to keep the original whole file sourcing, as it populates the envir for the functions.

@trestletech
Copy link
Contributor

Great point. My memory's a little fuzzy on this, but I do actually prefer the current behavior. And whether we argue that it's a feature or a bug, in the interest of maintaining backwards compatibility, it's probably a good idea to keep it.

If this isn't already tested, it might be worth covering. And that doc section I reference would probably be improved by mentioning this behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: question Poses a question, not a problem
Projects
None yet
Development

No branches or pull requests

2 participants