Open ide opened 8 years ago
FWIW I wrote a wrapper for react-helmet that does this:
import assert from 'assert';
import Helmet from 'react-helmet';
let isRendering = false;
function renderStatic(render) {
assert(
!isRendering,
`Must not call DocumentHead.renderStatic when already rendering an element`,
);
let head;
let result;
isRendering = true;
try {
result = render();
} finally {
head = Helmet.rewind();
isRendering = false;
}
return { head, result };
}
export default { renderStatic };
I like this. Could this also be opt-in so that .rewind() is still possible?
Are y'all still interested in this feature?
After reading @amannn's comment in react-helmet
, I understand why the rewind approach could be a shortcoming for some users, with the possible underlying race condition for parallel renders. The proposed API wouldn't resolve that, though. You would need to make separate side-effect wrapped components for each render/request to keep the internal state separate.
+1
Yeah, I think this is mostly independent from supporting concurrent rendering but writing out some hypothetical APIs (especially since React Fiber makes concurrent rendering more relevant) has shifted my opinion a bit.
The closure API would go from:
DocumentTitle.renderStatic(
() => ReactDOM.renderToString(<App />)
)
and could receive a context value:
// May be time-sliced over multiple rIC/rAF/microtask ticks
DocumentTitle.renderStaticAsync(
async (sideEffectState) => ReactDOM.renderToStringWithFiberScheduling(
// Haven't really thought about what goes on in here, probably something with React context
<App documentTitleSideEffectState={sideEffectState} />
)
)
In contrast, the rewind()
API instead would look look like:
let sideEffectState = DocumentTitle.createSideEffectState();
await ReactDOM.renderToStringWithFiberScheduling(
<App documentTitleSideEffectState={sideEffectState} />
);
// No more rewind() since all state is in sideEffectState, which gets garbage collected
I kind of like the simplicity of the latter API. I think if react-side-effect were to make a breaking change to its API and require explicitly using sideEffectState
and remove rewind()
, I'd prefer the latter API. It's clearer to see when different parts of the code run and you can't mess it up -- you can't silently forget to call createSideEffectState()
the same way you can forget to call rewind()
.
This is a proposal to deprecate the current API for server-side rendering:
and replace it with this, inspired by Aphrodite's SSR API:
This API has two advantages. It is impossible to forget to call
rewind
. It is also easy to throw an error ifDocumentTitle.renderStatic
is called in a reentrant way.You could use this with Aphrodite easily too. The
markup
field in the result is whatever you return from the closure (can figure out a better name later -- I just didn't want to call it "html" since it might include CSS):This is easy to build in user-space but I think it might be a strictly better API than
rewind()
so wanted to start a discussion here.