Skip to content

Add encoder argument to JSONResponse#2223

Closed
alex-oleshkevich wants to merge 1 commit intoKludex:masterfrom
alex-oleshkevich:json-encode
Closed

Add encoder argument to JSONResponse#2223
alex-oleshkevich wants to merge 1 commit intoKludex:masterfrom
alex-oleshkevich:json-encode

Conversation

@alex-oleshkevich
Copy link
Contributor

@alex-oleshkevich alex-oleshkevich commented Jul 22, 2023

This small change allows users to serialize a custom types with a little effort compared to subclassing.

import datetime
import json

from starlette.responses import JSONResponse


class MyJSONEncoder(json.JSONEncoder):
    def default(self, value):
        if isinstance(value, datetime.datetime):
            return value.isoformat()
        ...

async def app(scope, receive, send):
    assert scope['type'] == 'http'
    response = JSONResponse({'now': datetime.datetime.now()}, encoder=MyJSONEncoder)
    await response(scope, receive, send)

Similar to Django - https://docs.djangoproject.com/en/4.2/ref/request-response/#jsonresponse-objects

@Kludex
Copy link
Owner

Kludex commented Jul 22, 2023

We already had this discussion before... Need to find the reference.

But... I'm not keen on this change. It's too easy for the user to implement its custom Response class.

@alex-oleshkevich
Copy link
Contributor Author

I found it cleaner to share the encoder (when used not only for the API JSON responses) than building a custom response class. Moreover, inheriting the base class requires you to completely override JSON serialization, which may be unwanted in cases when base JSONResponse class does more than just json.dumps (not a case for now).

@Kludex
Copy link
Owner

Kludex commented Dec 1, 2023

@Kludex
Copy link
Owner

Kludex commented Dec 16, 2023

This PR goes against what is written in the "Custom JSON serialization" section in the documentation (the above section).

@alex-oleshkevich
Copy link
Contributor Author

ok, closing it

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