shinydisconnect icon indicating copy to clipboard operation
shinydisconnect copied to clipboard

fix cache

Open lz100 opened this issue 3 years ago • 4 comments

lz100 avatar Nov 27 '20 02:11 lz100

Thanks for the detailed issue report and taking the time to submit a PR! I really appreciate it. I haven't yet looked at the PR, I'll have to take the time to look into the code more closely, but just from reading the steps you describe in the issue it sounds good.

  • I understand you're adding a timer, can you confirm that this timer will not be always active and will always get deleted soon after the app is started? I want to ensure there isn't any performance degradation by having an infinitely running timer.

  • Has this solution been tested in multiple scenarios, such as: running locally in RStudio and killing the app vs running in a shiny server and stopping the server, made sure the error is seen whether the server dies or whether an error in the app is encountered

  • Regarding the last point about the demo app: It's true that the demo app is doing something funky and I'm sure you understand why. Even though this isn't the expected way to use the package, I do like the fact that {shinydisconnect} still works even in this odd case of dynamically inserting and immediately killing the app. Does this also mean that any app that gets errored out within 3 seconds will also have a similar issue?

daattali avatar Nov 30 '20 03:11 daattali

  • I use clearInterval to stop the timer. I tested and see it indeed stopped. You can try to add a console.log(num) between line 29 and 30 to see if it still runs.
  • I tested all your examples locally and deploy your demo to my account and they all worked for me.
  • Not all apps, only apps insert the script at the last second. To have my js function to run, it has to meet 2 requirements: 1. script inserted; 2. document ready. Usually when people use this package, script inserted into shiny UI and then the server runs, 1 -> 2. Your demo's server runs first and then script inserted when app stops, 2 -> 1. So scripts meet the requirements after the server ends and then start to run. So, 'shiny:disconnected' will first trigger error message, and then the timer cannot find server and report no connection.

lz100 avatar Nov 30 '20 16:11 lz100

Thanks, could you also add an entry to the NEWS file please

daattali avatar Dec 02 '20 18:12 daattali

Done. I check all examples again, no problem, but didn't deploy to check remotely this time.

lz100 avatar Dec 03 '20 00:12 lz100