Showcase filenames

[cpsievert]

(Best to review commit by commit)

The first commit fixes #1926

The second commit fixes what seems to be an undocumented problem when running an app file that is named something other than app.R in showcase mode (the app file should be 'active', that is, shown by default).

See rstudio/shiny-examples#157 for a testing app illustrating both of these fixes.

(BTW, we could use this testing app for #2582 as well, if we wanted)

[trestletech]

Overall, looks great! I just had a couple of minor questions/concerns.

[trestletech]

One super-nitpicky question, but LGTM! 💯

[wch]

There's a danger in setting .globals$appFile in this function. It's possible for this to happen:

• This function is called and the returned shiny app object is saved (but not run).
• Another app is run and then stops.
• The first app object is run.

In this scenario, then when the original app is running, the value of .globals$appFile will be from the other app.

A similar problem can happen if:

• An shiny app is run (and calls this function)
• Another shiny app, one that is generated by a function (and does not call this function) is run.

Then that second app will have .globals$appFile from the first app.

So I think we need to store this information in another way.

[cpsievert]

Would shinyOptions() be a decent way to ensure the filename is local to each app? Looks like we already do that to set/get the app directory here

shiny/R/app.R

Line 95 in 4b2e6fc

shinyOptions(appDir = getwd())

[wch]

I think this will still be applied at the global level, so it could affect subsequent apps in the same was as .globals$appFile would. This is because shinyAppDir could be called directly (outside of runApp, which creates a new layer on the options stack).

runApp adds a new layer on the stack here:

shiny/R/server.R

Lines 747 to 752 in b889b0d

	# Enable per-app Shiny options, for shinyOptions() and getShinyOption().
	oldOptionSet <- .globals$options
	on.exit({
	.globals$options <- oldOptionSet
	},add = TRUE)

So what I think is needed is to attach this information to the app object directly. In order to do that, I think we'll want to change shiny app objects so that they have a designated place to store extra information like this.

[cpsievert]

So what I think is needed is to attach this information to the app object directly.

I can go in that direction if you prefer, but what about setting this option inside of shinyAppDir_appR() and shinyAppDir_serverR()? I only bring it up because it'd be more consistent with what's already done here with the appDir in shinyAppDir_serverR:

shiny/R/app.R

Line 224 in b889b0d

shinyOptions(appDir = appDir)

That said, it's interesting/concerning that shinyAppDir_appR doesn't set the same option...

[cpsievert]

In the case that we attach this info to the app object, would you prefer a new undocumented option? Or a new top-level field?

[cpsievert]

Oh interesting, looks like you considered attaching appDir in a new appConfig field for bookmarkable state, but eventually decided against it, any idea why? f78bd08

[CLAassistant]

All committers have signed the CLA.