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

Should smart constructors for Rep* types in Yesod.Core.Content be marked as deprecated? #1780

Open
danbroooks opened this issue Sep 14, 2022 · 2 comments

Comments

@danbroooks
Copy link

danbroooks commented Sep 14, 2022

Currently, the functions repJson, repPlain and repXml in Yesod.Core.Content have no documenting comments:

repJson :: ToContent a => a -> RepJson
repJson = RepJson . toContent
repPlain :: ToContent a => a -> RepPlain
repPlain = RepPlain . toContent
repXml :: ToContent a => a -> RepXml
repXml = RepXml . toContent

However when using these today and googling for information about the right way of using them, I came across this stack overflow question, which has an answer mentioning that this functionality is deprecated:

https://stackoverflow.com/a/17282601/2061783

My understanding is that the whole Rep system is deprecated in Yesod 1.2, so Handler's now return Html and Value rather than RepHtml and RepJson.

A comment then goes on to link this changelog which mentions in 1.2:

Overhaul to the content/representation breakdown. RepHtml et al are deprecated. ChooseRep is gone. Now we have Content, TypedContent, and HasContentType. NOTE: This needs more explanation, I'll write a blog post about this and the YesodRequest/YesodResponse switch.

So my thought was it would make sense to have access to this information on these functions (where I started) in the docs rather than having to get to StackOverflow.

If these functions are effectively deprecated by way of the types they construct being deprecated then I think we should have a deprecation warning for these, along with some comments explaining why they are deprecated and a way of addressing that.

If someone can point me in this direction, I am happy to do the change request to add this documentation, I just need to know what the appropriate suggestion is instead (or if there is some existing documentation that could be linked here)

@schoettl
Copy link
Contributor

I quickly looked up my usage of this system. I also use the recommended Handler TypedContent and selectRep $ provideJson result where the type of result derives the ToJSON type class.

So for me, it sounds reasonable to deprecate the Rep* types and smart constructors and add documentation.

@snoyberg
Copy link
Member

I think it's possible there are still cases where using the Rep* types is more convenient than alternatives, but honestly I can't think of specific cases right now. A PR including docs for this and pointing to SO and this discussion are definitely welcome. A PR marking them as deprecated is probably good too, though I'm slightly hesitant.

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

No branches or pull requests

3 participants