useFormState: Emit comment to mark whether state matches #27307
Conversation
facebook-github-bot
added
CLA Signed
React Core Team
acdlite
force-pushed
the
useformstate-hydration
branch
from
6328da1
to
7fd3f6d
Compare
acdlite
force-pushed
the
useformstate-hydration
branch
from
7fd3f6d
to
56b6c00
Compare
| if (getIsHydrating()) { | ||
| // TODO: If this function returns true, it means we should use the form | ||
| // state passed to hydrateRoot instead of initialState. | ||
| tryToClaimNextHydratableFormMarkerInstance(currentlyRenderingFiber); | ||
| } |
There was a problem hiding this comment.
This hydrating gate and the one in tryToClaim... are redundant. I assume there will be other logic in here so probably can drop the one in tryTo...
There was a problem hiding this comment.
Yeah I did it that way to match the other similar functions in that file, which all have the guard at the top and also is redundant because the callers usually end up checking it regardless. Usually if I'm not the one who wrote a module I copy the style of the surrounding code for 1) consistency because I figure if I fix it here I should also fix it everywhere else 2) out of caution in case there was a good reason that I'm not thinking of :D
| export function canHydrateFormStateMarker( | ||
| instance: HydratableInstance, | ||
| ): null | FormStateMarkerInstance { | ||
| const nodeData = (instance: any).data; | ||
| if ( | ||
| nodeData === FORM_STATE_IS_MATCHING || | ||
| nodeData === FORM_STATE_IS_NOT_MATCHING | ||
| ) { | ||
| const markerInstance: FormStateMarkerInstance = (instance: any); | ||
| return markerInstance; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
There is this unfortunate forking of behavior in the canHydrate... functions where if we are in a Singleton like <body> or are in the container scope we assume there may be 3rd party nodes we need to skip over. If our nextHydratable was some injected 3rd party stylesheet then hydration would fail as this is currently implemented. I think this function needs. a similar treatment as canHydrateSuspenseInstance
This will make your optimization where we omit the marker when the hook is normal harder too b/c it'll need to be able to scan forward but also jump back to where it was if it doesn't find anything
There was a problem hiding this comment.
Re: the last point about the optimization, the idea is we wouldn't look for these nodes at all in that case. Basically it would revert to how it is today.
There was a problem hiding this comment.
hmm I think this would still be a problem
function App() {
... = useFormState();
return <div>...</div>
}
<script injectedBy3rdParty />
<!--!F-->
<div>
...
</div>
If I'm hydrating App the nextHydratableInstance will end up on <script injectedBy3rdParty /> Then when we check whether there is a form marker we see there isn't and we assume no postback state. Then we try to claim the next hydratable for <div>. We end up hitting the form state marker and stop causing a hydration mismatch.
There was a problem hiding this comment.
yeah that part makes sense, I just mean in the case where there's no postback state, there are no extra comment nodes at all, so all this code gets skipped anyway
| if (bufferedChunks !== null) { | ||
| // This component emitted some chunks. They are buffered until it finishes | ||
| // rendering successfully. We can flush them now. | ||
| const task: Task = (currentlyRenderingTask: any); | ||
| const target = task.blockedSegment.chunks; | ||
| // Not sure why Flow complains. If I change it to spread instead of apply, | ||
| // it checks fine. | ||
| // $FlowFixMe[method-unbinding] | ||
| target.push.apply(target, bufferedChunks); | ||
| } |
There was a problem hiding this comment.
I'm not sure this is going to work in the edge case
function App() {
let ... = useFormState();
return (
<html>
<body>
<form ...>
</body>
</html>
}
In Fizz (with float enabled) the html chunks are sequester so the fact that the comment is getting pushed to the target before the DOCTYPE does accidentally works out (we'll still emit <DOCTYPE html><html><head><!--F-->). However when hydrating on the client the starting point for hydration is document.body I believe (I can't remember if I made this change or I just talked about making it but conceptually we can start hydration from body b/c everything else is singleton which has it's own hydration path). If hydration starts from the body the form comment will be missed b/c it is in the head.
I don't have a good solution to this problem. We could maybe figure out if we need to put the comment before the <html> in the DOM by slotting it just after the DOCTYPE. Then we'd have to also special case the hydration cursor to look there. Additionally I think this may conflict with prerendering b/c we will have a static preamble (the html, head tag, and some head content) even when we can't fully prerender the entire shell. So we may have something dynamic (the form state indictator comment) that needs to be embedded inside the static part which just can't happen
There was a problem hiding this comment.
Another crude way to do it could be to forbid useFormState outside of the body 😆
| @@ -187,6 +193,8 @@ const SUSPENSE_START_DATA = '$'; | |||
| const SUSPENSE_END_DATA = '/$'; | |||
| const SUSPENSE_PENDING_START_DATA = '$?'; | |||
| const SUSPENSE_FALLBACK_START_DATA = '$!'; | |||
| const FORM_STATE_IS_MATCHING = '!F'; | |||
There was a problem hiding this comment.
Nit: I understand you're using this to mean NOT form state but the pattern that the other signaling comments use is:
TAG + STATE
So you can just check the first character to see if it matches the type of thing and then the second character is the state of that thing.
We should probably stick to that pattern. In theory if we did like switch(data[0]) it could matter.
| @@ -187,6 +193,8 @@ const SUSPENSE_START_DATA = '$'; | |||
| const SUSPENSE_END_DATA = '/$'; | |||
| const SUSPENSE_PENDING_START_DATA = '$?'; | |||
| const SUSPENSE_FALLBACK_START_DATA = '$!'; | |||
| const FORM_STATE_IS_MATCHING = '!F'; | |||
There was a problem hiding this comment.
| const FORM_STATE_IS_MATCHING = '!F'; | |
| const FORM_STATE_IS_MATCHING = 'F!'; |
acdlite
force-pushed
the
useformstate-hydration
branch
from
56b6c00
to
a4e47bf
Compare
acdlite
force-pushed
the
useformstate-hydration
branch
2 times, most recently
from
feb5892
to
d8844eb
Compare
A planned feature of useFormState is that if page load is the result of an MPA-style form submission — i.e. a form was submitted before it was hydrated, using Server Actions — the state should transfer to the next page. I haven't implemented that part yet, but as a prerequisite, we need some way for Fizz to indicate whether a useFormState hook was rendered using the "postback" state. That way we can do all state matching logic on the server without having to replicate it on the client, too. The approach here is to emit a comment node for each useFormState hook. We use one of two comment types: `<!--F-->` for a normal useFormState hook, and `<!--F!-->` for a hook that was rendered using the postback state. React will read these markers during hydration. This is similar to how we encode Suspense boundaries. Again, the actual matching algorithm is not yet implemented — for now, the "not matching" marker is always emitted. We can optimize this further by not emitting any markers for a render that is not the result of a form postback, which I'll do in subsequent PRs.
acdlite
force-pushed
the
useformstate-hydration
branch
from
d8844eb
to
18da28b
Compare
A planned feature of useFormState is that if the page load is the result of an MPA-style form submission — i.e. a form was submitted before it was hydrated, using Server Actions — the state of the hook should transfer to the next page. I haven't implemented that part yet, but as a prerequisite, we need some way for Fizz to indicate whether a useFormState hook was rendered using the "postback" state. That way we can do all state matching logic on the server without having to replicate it on the client, too. The approach here is to emit a comment node for each useFormState hook. We use one of two comment types: `<!--F-->` for a normal useFormState hook, and `<!--F!-->` for a hook that was rendered using the postback state. React will read these markers during hydration. This is similar to how we encode Suspense boundaries. Again, the actual matching algorithm is not yet implemented — for now, the "not matching" marker is always emitted. We can optimize this further by not emitting any markers for a render that is not the result of a form postback, which I'll do in subsequent PRs. DiffTrain build for [8b26f07](8b26f07)
React upstream changes: - facebook/react#27374 - facebook/react#27369 - facebook/react#27372 - facebook/react#27371 - facebook/react#27370 - facebook/react#27321 - facebook/react#27368 - facebook/react#27367 - facebook/react#27366 - facebook/react#27360 - facebook/react#27361 - facebook/react#27357 - facebook/react#27359 - facebook/react#27358 - facebook/react#27330 - facebook/react#27347 - facebook/react#27307 - facebook/react#27346 - facebook/react#27342 - facebook/react#27340 - facebook/react#27328 - facebook/react#27327 - facebook/react#27325 - facebook/react#27337 - facebook/react#27336 - facebook/react#27323 - facebook/react#27320 - facebook/react#27317 - facebook/react#27318 - facebook/react#27316 - facebook/react#27313 - facebook/react#27309 - facebook/react#27302 - facebook/react#27297 - facebook/react#27295 - facebook/react#27305 - facebook/react#27215 - facebook/react#27304 - facebook/react#27067 - facebook/react#27179 - facebook/react#27278 - facebook/react#27277 - facebook/react#27282 - facebook/react#27230 - facebook/react#27260 - facebook/react#27270 - facebook/react#27273 - facebook/react#27268 - facebook/react#27269
React upstream changes: - facebook/react#27374 - facebook/react#27369 - facebook/react#27372 - facebook/react#27371 - facebook/react#27370 - facebook/react#27321 - facebook/react#27368 - facebook/react#27367 - facebook/react#27366 - facebook/react#27360 - facebook/react#27361 - facebook/react#27357 - facebook/react#27359 - facebook/react#27358 - facebook/react#27330 - facebook/react#27347 - facebook/react#27307 - facebook/react#27346 - facebook/react#27342 - facebook/react#27340 - facebook/react#27328 - facebook/react#27327 - facebook/react#27325 - facebook/react#27337 - facebook/react#27336 - facebook/react#27323 - facebook/react#27320 - facebook/react#27317 - facebook/react#27318 - facebook/react#27316 - facebook/react#27313 - facebook/react#27309 - facebook/react#27302 - facebook/react#27297 - facebook/react#27295 - facebook/react#27305 - facebook/react#27215 - facebook/react#27304 - facebook/react#27067 - facebook/react#27179 - facebook/react#27278 - facebook/react#27277 - facebook/react#27282 - facebook/react#27230 - facebook/react#27260 - facebook/react#27270 - facebook/react#27273 - facebook/react#27268 - facebook/react#27269
React upstream changes: - facebook/react#27374 - facebook/react#27369 - facebook/react#27372 - facebook/react#27371 - facebook/react#27370 - facebook/react#27321 - facebook/react#27368 - facebook/react#27367 - facebook/react#27366 - facebook/react#27360 - facebook/react#27361 - facebook/react#27357 - facebook/react#27359 - facebook/react#27358 - facebook/react#27330 - facebook/react#27347 - facebook/react#27307 - facebook/react#27346 - facebook/react#27342 - facebook/react#27340 - facebook/react#27328 - facebook/react#27327 - facebook/react#27325 - facebook/react#27337 - facebook/react#27336 - facebook/react#27323 - facebook/react#27320 - facebook/react#27317 - facebook/react#27318 - facebook/react#27316 - facebook/react#27313 - facebook/react#27309 - facebook/react#27302 - facebook/react#27297 - facebook/react#27295 - facebook/react#27305 - facebook/react#27215 - facebook/react#27304 - facebook/react#27067 - facebook/react#27179 - facebook/react#27278 - facebook/react#27277 - facebook/react#27282 - facebook/react#27230 - facebook/react#27260 - facebook/react#27270 - facebook/react#27273 - facebook/react#27268 - facebook/react#27269
### React upstream changes: - facebook/react#27374 - facebook/react#27369 - facebook/react#27372 - facebook/react#27371 - facebook/react#27370 - facebook/react#27321 - facebook/react#27368 - facebook/react#27367 - facebook/react#27366 - facebook/react#27360 - facebook/react#27361 - facebook/react#27357 - facebook/react#27359 - facebook/react#27358 - facebook/react#27330 - facebook/react#27347 - facebook/react#27307 - facebook/react#27346 - facebook/react#27342 - facebook/react#27340 - facebook/react#27328 - facebook/react#27327 - facebook/react#27325 - facebook/react#27337 - facebook/react#27336 - facebook/react#27323 - facebook/react#27320 - facebook/react#27317 - facebook/react#27318 - facebook/react#27316 - facebook/react#27313 - facebook/react#27309 - facebook/react#27302 - facebook/react#27297 - facebook/react#27295 - facebook/react#27305 - facebook/react#27215 - facebook/react#27304 - facebook/react#27067 - facebook/react#27179 - facebook/react#27278 - facebook/react#27277 - facebook/react#27282 - facebook/react#27230 - facebook/react#27260 - facebook/react#27270 - facebook/react#27273 - facebook/react#27268 - facebook/react#27269
) A planned feature of useFormState is that if the page load is the result of an MPA-style form submission — i.e. a form was submitted before it was hydrated, using Server Actions — the state of the hook should transfer to the next page. I haven't implemented that part yet, but as a prerequisite, we need some way for Fizz to indicate whether a useFormState hook was rendered using the "postback" state. That way we can do all state matching logic on the server without having to replicate it on the client, too. The approach here is to emit a comment node for each useFormState hook. We use one of two comment types: `<!--F-->` for a normal useFormState hook, and `<!--F!-->` for a hook that was rendered using the postback state. React will read these markers during hydration. This is similar to how we encode Suspense boundaries. Again, the actual matching algorithm is not yet implemented — for now, the "not matching" marker is always emitted. We can optimize this further by not emitting any markers for a render that is not the result of a form postback, which I'll do in subsequent PRs.
A planned feature of useFormState is that if the page load is the result of an MPA-style form submission — i.e. a form was submitted before it was hydrated, using Server Actions — the state of the hook should transfer to the next page. I haven't implemented that part yet, but as a prerequisite, we need some way for Fizz to indicate whether a useFormState hook was rendered using the "postback" state. That way we can do all state matching logic on the server without having to replicate it on the client, too. The approach here is to emit a comment node for each useFormState hook. We use one of two comment types: `<!--F-->` for a normal useFormState hook, and `<!--F!-->` for a hook that was rendered using the postback state. React will read these markers during hydration. This is similar to how we encode Suspense boundaries. Again, the actual matching algorithm is not yet implemented — for now, the "not matching" marker is always emitted. We can optimize this further by not emitting any markers for a render that is not the result of a form postback, which I'll do in subsequent PRs. DiffTrain build for commit 8b26f07.
A planned feature of useFormState is that if the page load is the result of an MPA-style form submission — i.e. a form was submitted before it was hydrated, using Server Actions — the state of the hook should transfer to the next page.
I haven't implemented that part yet, but as a prerequisite, we need some way for Fizz to indicate whether a useFormState hook was rendered using the "postback" state. That way we can do all state matching logic on the server without having to replicate it on the client, too.
The approach here is to emit a comment node for each useFormState hook. We use one of two comment types:
<!--F-->for a normal useFormState hook, and<!--F!-->for a hook that was rendered using the postback state. React will read these markers during hydration. This is similar to how we encode Suspense boundaries.Again, the actual matching algorithm is not yet implemented — for now, the "not matching" marker is always emitted.
We can optimize this further by not emitting any markers for a render that is not the result of a form postback, which I'll do in subsequent PRs.