preempt jsonlite message about keep_vec_names

[r2evans]

Prevent jsonlite's message about keep_vec_names=:

### without this patch
toJSON(list(a=5, b=quantile(1:5, 0.5)))
# Input to asJSON(keep_vec_names=TRUE) is a named vector. In a future version of jsonlite, this option will not be supported, and named vectors will be translated into arrays instead of objects. If you want JSON object output, please use a named list instead. See ?toJSON.
# {"a":5,"b":{"50%":3}} 

### with this patch
toJSON(list(a=5, b=quantile(1:5, 0.5)))
# {"a":5,"b":{"50%":3}} 

If there are specific places where this is not working (and the use does not otherwise trigger the message), setting fix_vec_names=FALSE will bypass the additional step.

I do find many calls to jsonlite::toJSON(.) in the package in addition to the (internal) toJSON(.), I'm not sure if that's intentional or if there can be some dis-ambiguation here. I'm happy to participate in that if it is beneficial.

[gadenbuie]

@r2evans Thanks for this proposal, but it isn't a complete fix as you're only taking into account first-level objects.

## with this PR
toJSON(list(a=5, b=list(b1 = c(b1 = "one"))))
#> Input to asJSON(keep_vec_names=TRUE) is a named vector. In a future version of jsonlite,
#> this option will not be supported, and named vectors will be translated into arrays
#> instead of objects. If you want JSON object output, please use a named list instead.
#> See ?toJSON.
#> {"a":5,"b":{"b1":{"b1":"one"}}} 

There's a bunch of historical behavior here, in fact the keep_vec_names warning has been around since the beginning of jsonlite:

• Legacy wrapper with rjson/RJSONIO behaviour jeroen/jsonlite#73
• toJSON drops names of named vectors jeroen/jsonlite#76

Ideally, in the places where this error arises from internal Shiny code, we'd want to fix the warning. Feel free to open new issues with reproducible examples and we'll work to fix them.

Beyond that, this behavior has been around long enough that I think jsonlite should either accept that keep_vec_names = TRUE is behavior people do want to opt into and should be allowed without a warning; or jsonlite should follow through on the "will be not be supported" plan.

[r2evans]

you're only taking into account first-level objects

That's a great point! I can update for that.

There's a bunch of historical behavior here

Certainly! And it's never been fully addressed. I believe it is not just jsonlite's responsibility to look at this.

Ideally, in the places where this error arises from internal Shiny code, we'd want to fix the warning

Technically, since my code is only for the internal toJSON, that's where it is targeted. I'm still confused a bit by all of the mixed use of toJSON and jsonlite::toJSON, not sure if it's intentional or spiral-growth. Again, I'm happy to work towards that if it is an issue and/or will help with this local PR effort.

Feel free to open new issues

Besides #2673?

Beyond that, this behavior has been around long enough that I think jsonlite should either accept that keep_vec_names = TRUE is behavior people do want to opt into and should be allowed without a warning; or jsonlite should follow through on the "will be not be supported" plan.

I don't wholly disagree. (Ironic that while you're telling me jsonlite should resolve it, the original suggestion for this message was suggested by wch himself, not jeroen.)

Doing what is suggested (dropping support, and therefore dropping all names in vectors) is likely the best thing to do, but wch's comment in that issue clearly communicated that that is not (or was not) the desired path for shiny, so ... we come to the point that one can assume that shiny wants named vectors to continue. Suggesting otherwise is counter to the request and conversation that started this.

Frankly, I think there are a few paths from here:

1. wch and jeroen agree that keep_vec_names= should be ignored (though for backward-compatible code, not deleted): all named vectors will be unnamed, and shiny will adapt (by converting all nested named vectors to lists). I think for fairness and completeness, since wch is still prominent in the dev of shiny and the R community as a whole, it makes sense for wch to acknowledge or cede a major role in that discussion.
2. This PR be adapted to continue under the assumption that jsonlite may not change in the immediate future. Having a recursive unnaming is imperfect, but it reduces confusing (to many) comments and noise in the logging. It's more confusing because most of the thing producing this message are completely obscured/hidden/unreachable to the only people who actually see the message.
3. We change the PR to wrap all calls to jsonlite::toJSON with suppressMessages (or perhaps a more-precise withCallingHandlers(message=)) so that this message is avoided when triggered by shiny code nobody has visibilty to. This has the desired side-effect, though it only addresses a symptom and does not venture into the cause (or debate).

Would you be more supportive of the third option above? The unstated fourth option of status quo is what I'm trying to avoid.

Thoughts @gadenbuie ?

[gadenbuie]

It has been a long while since those comments and decisions were made. I opened jeroen/jsonlite#456 to discuss either moving forward on the promised plan or abandoning the message (my preference).

I appreciate your offer to update the PR, but I don't think we should get into the business of recursively traversing data; especially not in a place that's on the critical path between server and client communication.

I'd lean toward suppressing the message if jsonlite wants to keep the message indefinitely; otherwise I'd prefer to wait and see what jsonlite does.