Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: facebook/react
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: e56f4ae38d118168e0561f1b86ecbdef592138e4
Choose a base ref
...
head repository: facebook/react
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: d1afcb43fd506297109c32ff462f6f659f9110ae
Choose a head ref
  • 5 commits
  • 17 files changed
  • 1 contributor

Commits on Aug 30, 2024

  1. [Flight] Ship DEV-only enableServerComponentLogs flag in Stable/Canary (

    #30847)
    
    To recap. This only affects DEV and RSC. It patches console on the
    server in DEV (similar to how React DevTools already does and what we
    did for the double logging). Then replays those logs with a `[Server]`
    badge on the client so you don't need a server terminal open.
    
    This has been on for over 6 months now in our experimental channel and
    we've had a lot of coverage in Next.js due to various experimental flags
    like taint and ppr.
    
    It's non-invasive in that even if something throws we just serialize
    that as an unknown value.
    
    The main feedback we've gotten was:
    
    - The serialization depth wasn't deep enough which I addressed in #30294
    and haven't really had any issues since. This could still be an issue or
    the inverse that you serialize too many logs that are also too deep.
    This is not so much an issue with intentional logging and things like
    accidental errors don't typically have unbounded arguments (e.g. React
    errors are always string arguments). The ideal would be some way to
    retain objects and then load them on-demand but that needs more
    plumbing. Which can be later.
    - The other was that double logging on the server is annoying if the
    same terminal does both the RSC render and SSR render which was
    addressed in #30207. It is now off by default in node/edge-builds of the
    client, on by default in browser builds. With the `replayConsole` option
    to either opt-in or out.
    
    We've reached a good spot now I think.
    
    These are better with `enableOwnerStacks` but that's a separate track
    and not needed.
    
    The only thing to document here, other than maybe that we're doing it,
    is the `replayConsole` option but that's part of the RSC renderers that
    themselves are not documented so nowhere to document it.
    sebmarkbage authored Aug 30, 2024
    Configuration menu
    Copy the full SHA
    4f60494 View commit details
    Browse the repository at this point in the history

Commits on Sep 3, 2024

  1. [DevTools] Add Filtering of Environment Names (#30850)

    Stacked on #30842.
    
    This adds a filter to be able to exclude Components from a certain
    environment. Default to Client or Server.
    
    The available options are computed into a dropdown based on the names
    that are currently used on the page (or an option that were previously
    used). In addition to the hardcoded "Client". Meaning that if you have
    Server Components on the page you see "Server" or "Client" as possible
    options but it can be anything if there are multiple RSC environments on
    the page.
    
    "Client" in this case means Function and Class Components in Fiber -
    excluding built-ins.
    
    If a Server Component has two environments (primary and secondary) then
    both have to be filtered to exclude it.
    
    We don't show the option at all if there are no Server Components used
    in the page to avoid confusing existing users that are just using Client
    Components and wouldn't know the difference between Server vs Client.
    
    <img width="815" alt="Screenshot 2024-08-30 at 12 56 42 AM"
    src="https://github.com/user-attachments/assets/e06b225a-e85d-4cdc-8707-d4630fede19e">
    sebmarkbage authored Sep 3, 2024
    Configuration menu
    Copy the full SHA
    04ec50e View commit details
    Browse the repository at this point in the history
  2. [DevTools] Support VirtualInstances in findAllCurrentHostInstances (#…

    …30853)
    
    This lets us highlight Server Components.
    
    However, there is a problem with this because if the actual nearest
    Fiber is filtered, there's no FiberInstance and so we might skip past it
    and maybe never find a child while walking the whole tree. This is very
    common in the case where you have just Server Components and Host
    Components which are filtered by default.
    
    Note how the DOM nodes that are just plain host instances without client
    component wrappers are not highlighted here:
    
    <img width="1102" alt="Screenshot 2024-08-30 at 4 33 55 PM"
    src="https://github.com/user-attachments/assets/c9a7b91e-5faf-4c60-99a8-1195539ff8b5">
    
    Fixing that needs a separate refactor though and related to several
    other features that already have a similar issue without
    VirtualInstances like Suspense/Error Boundaries (triggering
    suspense/error on a filtered Suspense/ErrorBoundary doesn't work
    correctly). So this first PR just adds the feature for the common case
    where there's at least some Fibers.
    sebmarkbage authored Sep 3, 2024
    Configuration menu
    Copy the full SHA
    e0a07e9 View commit details
    Browse the repository at this point in the history
  3. [Fiber] Stash ThenableState on the Dependencies Object for Use By Dev…

    …Tools (#30866)
    
    This lets us track what a Component might suspend on from DevTools. We
    could already collect this by replaying a component's Hooks but that
    would be expensive to collect from a whole tree.
    
    The thenables themselves might contain useful information but mainly
    we'd want access to the `_debugInfo` on the thenables which might
    contain additional information from the server.
    
    
    https://github.com/facebook/react/blob/19bd26beb689e554fceb0b929dc5199be8cba594/packages/shared/ReactTypes.js#L114
    
    In a follow up we should really do something similar in Flight to
    transfer `use()` on the debugInfo of that Server Component.
    sebmarkbage authored Sep 3, 2024
    Configuration menu
    Copy the full SHA
    8d68da3 View commit details
    Browse the repository at this point in the history
  4. [DevTools] Track all public HostInstances in a Map (#30831)

    This lets us get from a HostInstance to the nearest DevToolsInstance
    without relying on `findFiberByHostInstance` and
    `fiberToDevToolsInstanceMap`. We already did the equivalent of this for
    Resources in HostHoistables.
    
    One issue before was that we'd ideally get away from the
    `fiberToDevToolsInstanceMap` map in general since we should ideally not
    treat Fibers as stateful but they could be replaced by something else
    stateful in principle.
    
    This PR also addresses Virtual Instances. Now you can select a DOM node
    and have it select a Virtual Instance if that's the nearest parent since
    the parent doesn't have to be a Fiber anymore.
    
    However, the other reason for this change is that I'd like to get rid of
    the need for the `findFiberByHostInstance` from being injected. A
    renderer should not need to store a reference back from its instance to
    a Fiber. Without the Synthetic Event system this wouldn't be needed by
    the renderer so we should be able to remove it. We also don't really
    need it since we have all the information by just walking the commit to
    collect the nodes if we just maintain our own Map.
    
    There's one subtle nuance that the different renderers do. Typically a
    HostInstance is the same thing as a PublicInstance in React but
    technically in Fabric they're not the same. So we need to translate
    between PublicInstance and HostInstance. I just hardcoded the Fabric
    implementation of this since it's the only known one that does this but
    could feature detect other ones too if necessary. On one hand it's more
    resilient to refactors to not rely on injected helpers and on hand it
    doesn't follow changes to things like this.
    
    For the conflict resolution I added in #30494 I had to make that
    specific to DOM so we can move the DOM traversal to the backend instead
    of the injected helper.
    sebmarkbage authored Sep 3, 2024
    Configuration menu
    Copy the full SHA
    d1afcb4 View commit details
    Browse the repository at this point in the history
Loading