[pull] main from facebook:main #273
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The previous PR added name hints for anonymous functions, but didn't handle the case of outlined functions. Here we do some cleanup around function `id` and name hints: * Make `HIRFunction.id` a ValidatedIdentifierName, which involved some cleanup of the validation helpers * Add `HIRFunction.nameHint: string` as a place to store the generated name hints which are not valid identifiers * Update Codegen to always use the `id` as the actual function name, and only use nameHint as part of generating the object+property wrapper for debug purposes. This ensures we don't conflate synthesized hints with real function names. Then, we also update OutlineFunctions to use the function name _or_ the nameHint as the input to generating a unique identifier. This isn't quite as nice as the object form since we lose our formatting, but it's a simple step that gives more context to the developer than `_temp` does. Switching to output the object+property lookup form for outlined functions is a bit more involved, let's do that in a follow-up.
One thing that can suspend is the downloading of the RSC stream itself. This tracks an I/O entry for each Promise (`SomeChunk<T>`) that represents the request to the RSC stream. As the value we use the `Response` for `createFromFetch` (or the `ReadableStream` for `createFromReadableStream`). The start time is when you called those. Since we're not awaiting the whole stream, each I/O entry represents the part of the stream up until it got unblocked. However, in a production environment with TLS packets and buffering in practice the chunks received by the client isn't exactly at the boundary of each row. It's a bit longer into larger chunks. From testing, it seems like multiples of 16kb or 64kb uncompressed are common. To simulate a production environment we group into roughly 64kb chunks if they happen in rapid sequence. Note that this might be too small to give a good idea because of the throttle many boundaries might be skipped anyway so this might show too many. The React DevTools will see each I/O entry as separate but dedupe if an outer boundary already depends on the same chunk. This deduping makes it so that small boundaries that are blocked on the same chunk, don't get treated as having unique suspenders. If you have a boundary with large content, then that content will likely be in a separate chunk which is not in the parent and then it gets marked as. This is all just an approximation. The goal of this is just to highlight that very large boundaries will very likely suspend even if they don't suspend on any I/O on the server. In practice, these boundaries can float around a lot and it's really any Suspense boundary that might suspend but some are more likely than others which this is meant to highlight. It also just lets you inspect how many bytes needs to be transferred before you can show a particular part of the content, to give you an idea that it's not just I/O on the server that might suspend. If you don't use the debug channel it can be misleading since the data in development mode stream will have a lot more data in it which leads to more chunking. Similarly to "client references" these I/O infos don't have an "env" since it's the client that has the I/O and so those are excluded from flushing in the Server performance tracks. Note that currently the same Response can appear many times in the same Instance of SuspenseNode in DevTools when there are multiple chunks. In a follow up I'll show only the last one per Response at any given level. Note that when a separate debugChannel is used it has its own I/O entry that's on the `_debugInfo` for the debug chunks in that channel. However, if everything works correctly these should never leak into the DevTools UI since they should never be propagated from a debug chunk to the values waited by the runtime. This is easy to break though.
This was fun. We previously added the MaybeAlias effect in #33984 in order to describe the semantic that an unknown function call _may_ alias its return value in its result, but that we don't know this for sure. We record mutations through MaybeAlias edges when walking backward in the data flow graph, but downgrade them to conditional mutations. See the original PR for full context. That change was sufficient for the original case like ```js const frozen = useContext(); useEffect(() => { frozen.method().property = true; }, [...]); ``` But it wasn't sufficient for cases where the aliasing occured between operands: ```js const dispatch = useDispatch(); <div onClick={(e) => { dispatch(...e.target.value) e.target.value = ...; }} /> ``` Here we would record a `Capture dispatch <- e.target` effect. Then during processing of the `event.target.value = ...` assignment we'd eventually _forward_ from `event` to `dispatch` (along a MaybeAlias edge). But in #33984 I missed that this forward walk also has to downgrade to conditional. In addition to that change, we also have to be a bit more precise about which set of effects we create for alias/capture/maybe-alias. The new logic is a bit clearer, I think: * If the value is frozen, it's an ImmutableCapture edge * If the values are mutable, it's a Capture * If it's a context->context, context->mutable, or mutable->context, count it as MaybeAlias.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.3)
Can you help keep this open source service alive? 💖 Please sponsor : )