JohnCoene / firebase

Google FIrebase for shiny
https://firebase.john-coene.com
GNU Affero General Public License v3.0
171 stars 26 forks source link

Clarification on use of `interactive` needed #76

Closed kennedymwavu closed 11 months ago

kennedymwavu commented 11 months ago

Background Info

I was going through the source code and stumbled upon this: image Link: https://github.com/JohnCoene/firebase/blob/b72e8847191d7826f0bfd204409682581cd8251e/R/class-auth.R#L280C25-L280C25

if (interactive) is used instead of the expected if (interactive()). I can't find any explicit definition for interactive as a variable. This pattern is repeated multiple times so I'd assume it's not a bug.

Question

Is this a unique R feature or am I overlooking something?

Expectation

My expectation of if (interactive) is that it should throw an error since interactive is a function, not a logical. And, indeed, it is what happens in my reprex below:

check_interactive_mode <- \() {
  if (interactive) {
    print("This is an interactive session")
  } else {
    print("This is a non-interactive session")
  }
}

check_interactive_mode()
#> Error in if (interactive) {: argument is not interpretable as logical

Created on 2023-12-13 with reprex v2.0.2

JohnCoene commented 11 months ago

Thank you for this.

It's clearly a bug from @munoztd0 :p

But, because this runs in a shiny server interactive() will never be true. This has to be reworked

munoztd0 commented 11 months ago

yeah my bad I fixed the typo and now I understand why it worked when it should have not..

munoztd0 commented 11 months ago

Removed my mistake in the commit fad6e21.

To re explain the issue is that Windows internal clock didn't align with the API so that would block people developing on the windows machine.

I just added a way to bypass this error for development purposes but it should be fine once in prod on a linux server. I don't know about a Windows server though don't really have anything to really test it and I don't think it woud come up very often ..

munoztd0 commented 11 months ago

I will close this one and reopen the main one #64