feat: Decouple rendering from async context #842
Conversation
| export * from "../render/renderToStream"; | ||
| export * from "../render/renderToString"; | ||
| export * from "../requestInfo/types"; | ||
| export * from "../requestInfo/utils"; |
There was a problem hiding this comment.
Debating if we should expose this to userland, indifferent tbh
There was a problem hiding this comment.
Wondering same 👍 I think we can keep it exposed and defer documenting it for now.
| @@ -0,0 +1,54 @@ | |||
| import { FC } from "react"; | |||
There was a problem hiding this comment.
Decided to make a new file for this since it contains jsx
| ): RequestInfo => { | ||
| const { rw: rwOverrides, ...otherRequestInfoOverrides } = overrides; | ||
|
|
||
| const defaultRequestInfo: RequestInfo = { |
There was a problem hiding this comment.
Copied over from defineApp, not sure if these defaults are good though
| * Constructs a generic requestInfo that can be used as defaults. | ||
| * Allows for passing in overrides to initialize with defaults. | ||
| */ | ||
| export const constructWithDefaultRequestInfo = ( |
There was a problem hiding this comment.
Can potentially use this elsewhere like defineApp
There was a problem hiding this comment.
Thinking bout this more, probably just keeping it as getDefaultRequestInfo will be better
There was a problem hiding this comment.
Looks good as is! We likely do want to be doing a custom merge (e.g. allowing forrw overrides) as you're already doing
b3c3baf to
545d862
Compare
| waitUntil: () => {}, | ||
| passThroughOnException: () => {}, | ||
| props: {}, | ||
| }, |
There was a problem hiding this comment.
Ah, I guess we need this stubbed this way since the caller wouldn't be providing this, makes sense 👍
|
Closing and re-opening to get CI builds to run, sorry for the noise |
|
This looks great, thank you for taking this on @gching! And I appreciate the detailed context/writeup for the PR description Just getting the CI builds running now, will merge and release once green |
Ref #840
The current rendering functions (
renderToStream,renderToString) are tightly coupled to therequestInfoobject provided by async local storage. This makes it difficult to use our rendering engine in environments outsidedefineApp.This PR decouples the rendering logic from this implicit context.
Changes
constructWithDefaultRequestInfo: Introduced and exposes a new utility function in entry fromsdk/src/runtime/requestInfo/utils.tsx. This function creates a defaultRequestInfoobject and merging with user-provided overrides.renderToStreamandrenderToStringwere updated to accept an optionalrequestInfoparameterBefore Change
renderToStreamimplicitly depended on therequestInfofrom the async local storage contextrunWithRequestInfofunction and manually construct a completeRequestInfoobject.After Change
requestInfoobjectrequestInfoand overrides it given the contextrequestInfo(backwards compatible) if it exists and user providedrequestInfooverridesconstructWithDefaultRequestInfoto be used to construct arequestInfoif needed (ie. we can use this indefineApppotentially)