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

Support JSON.stringify of objects #77

Merged
merged 1 commit into from
Dec 21, 2023

Conversation

yogat3ch
Copy link
Contributor

Hi @jonthegeek!
I was wondering if cookies can support JSON.stringify of objects passed as the cookie_value? This will allow for a simplification of usage where a single app cookie object with a bunch of values can be written/read.

@jonthegeek
Copy link
Contributor

I definitely like the idea, but I don't think I like where you put it. That should be a fairly dumb bridge between the R code and the JS code. I want to make sure we're doing this in a place where we can control the error messaging (and, eventually, logging).

That said, I'd rather have it and then refactor, than not have it at all. Looking at the code, the place I'd put it isn't cleanly available right now; I really need to do some work on this package overall!

Soooo.... ok, I'll go ahead and accept this, but I'm writing an issue or three first to make sure I fix it 🙃

Copy link
Contributor

@jonthegeek jonthegeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I'd prefer that this be handled differently (see #78), this is definitely an incremental improvement, and the handling should remain invisible to the user even after I refactor.

Thanks for the idea!

@jonthegeek jonthegeek enabled auto-merge (squash) December 21, 2023 17:39
@jonthegeek jonthegeek disabled auto-merge December 21, 2023 19:18
@jonthegeek jonthegeek merged commit 8dec8b4 into shinyworks:main Dec 21, 2023
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants