Proposal: Refactor render to create InertiaResponse class #61
Conversation
@BrandonShar, I'm putting aside some time later this week to do some work on this project, if you are ok to wait for a couple of days. |
svengt
left a comment
svengt
left a comment
There was a problem hiding this comment.
@BrandonShar Very valuable refactor! In my projects I like to use marshmallow to serialize my props. With this change I can easily add this logic by overriding the build_props method in a custom class.
| 'clearHistory': clear_history, | ||
| } | ||
| class InertiaResponse(HttpResponse, BaseInertiaResponseMixin): | ||
| json_encoder = settings.INERTIA_JSON_ENCODER |
There was a problem hiding this comment.
Maybe this attribute should be moved to the mixin, as it depends on it?
| response.raise_for_status() | ||
| return response.json(), INERTIA_SSR_TEMPLATE | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Would be nice to introduce a logger here. This way people can monitor issues with the SSR service.
There was a problem hiding this comment.
This is a good idea! Does Django have something standard or would we need something extensible here?
Either way, let's break this idea out into a separate PR.
There was a problem hiding this comment.
Good idea to break it into a separate PR.
Python has a built in solution for this. This is an example implementation:
import logging
logger = logging.getLogger(__name__)
...
def build_first_load_context_and_template(self, data):
if settings.INERTIA_SSR_ENABLED:
try:
...
except Exception:
logger.exception("Error while calling ssr render endpoint.")
There was a problem hiding this comment.
I can second this. Python's logger is the standard
| _page = { | ||
| 'component': self.component, | ||
| 'props': self.build_props(), | ||
| 'url': self.request.get_full_path(), |
Issue #33 mentioned that
renderbeing implemented as a method was restricting customization and as I thought about #54 I realized that having a class based response would allow us to better utilize existing Django patterns for creating class based inertia views.So this refactor would both allow safer customizations in userland AND lay better groundwork for embracing more django features.
Setting this up also required changing how we mock the built in tests, but I think this actually makes them much more straightforward (and work more often! 😄 )
Feedback is super welcome!