-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add request context and props to custom renderer #373
Conversation
I'm really curious about how you handle i18n, if you don't mind giving some details, I was thinking about sending the texts from the view since #320 was merged, a bit annoying, I agree, but it would allow me to use django-rosetta for all texts so translators can translate directly in the admin. |
@andreighh We have a context processor based on Django's JSON Catalog view. Something like this: from typing import List, TypedDict, Union
from django.http import HttpRequest
from django.utils.translation import get_language
from django.utils.translation.trans_real import DjangoTranslation
from django.views.i18n import JSONCatalog
class CatalogEntry(TypedDict):
key: str
value: str
class TranslationCatalog(TypedDict):
language: str
catalog: List[CatalogEntry]
class TranslationProcessor(TypedDict):
i18n: TranslationCatalog
def translation(request: HttpRequest) -> TranslationProcessor:
catalog = JSONCatalog()
locale: str = get_language()
catalog.translation = DjangoTranslation(locale, domain=catalog.domain)
context = catalog.get_context_data()
return TranslationProcessor(
i18n=TranslationCatalog(
language=locale,
catalog=[
CatalogEntry(
key=k,
value=v,
)
for k, v in context["catalog"].items()
],
),
) That gets our JS translations into context. Then we can use them with a translation function like ttag or your own implementation of Django's |
Thank you! |
@silviogutierrez Any thoughts/concerns on this? |
@crgwbr : I added comments/changes a while back. Did GitHub mess up the notifications? |
@silviogutierrez I don't see any comments anywhere on here. Must have gotten lost somewhere. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, so "Pending" means "pending submission" not "pending fixes".
packages/reactivated/src/conf.tsx
Outdated
@@ -6,7 +6,13 @@ export type Options = { | |||
client?: (options: ClientConfig) => InlineConfig; | |||
renderer?: (options: RendererConfig) => InlineConfig; | |||
}; | |||
render?: (content: JSX.Element) => Promise<JSX.Element>; | |||
render?: ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we actually use the rest of the Options object, looking around. Just Renderer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Done
context: any; | ||
props: any; | ||
}, | ||
) => Promise<JSX.Element>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Renderer should be:
- Generic, `Renderer<T = unknown>'.
- Context should be T, props should always be unknown.
- In the library code, use Renderer without a generic param.
- But in the custom implementation, have
@reactivated
/generator.mts
outputRenderer
bound asRenderer<_Types["Context"]>
Hope that makes sense.
My client/renderer.tsx
looks like this, for reference, so the generic would be seamless and type if further:
import React from "react";
import {Renderer} from "@reactivated";
export default (async (content) => {
const {makeStore} = await import("@client/store");
const store = makeStore();
const {Provider} = await import("react-redux");
const {resetServerContext} = await import("react-beautiful-dnd");
resetServerContext();
return <Provider store={store}>{content}</Provider>;
}) satisfies Renderer;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Done
6049424
to
810c01b
Compare
810c01b
to
76e5d66
Compare
Ran into a case where I need request context data in our custom SSR renderer function. Specifically, I need this for loading translated strings from context into ttag—but this seems like it could be pretty broadly useful beyond that one use case.