Skip to content
Snippets Groups Projects
This project is mirrored from https://github.com/facebook/react.git. Pull mirroring updated .
  1. Oct 08, 2024
  2. Oct 04, 2024
  3. Oct 03, 2024
    • Mofei Zhang's avatar
      [compiler] Optional chaining for dependencies (HIR rewrite) · 0751fac7
      Mofei Zhang authored
      Adds HIR version of `PropagateScopeDeps` to handle optional chaining.
      
      Internally, this improves memoization on ~4% of compiled files (internal links: [1](https://www.internalfb.com/intern/paste/P1610406497/))
      
      Summarizing the changes in this PR.
      1. `CollectOptionalChainDependencies` recursively traverses optional blocks down to the base. From the base, we build up a set of `baseIdentifier.propertyA?.propertyB` mappings.
      The tricky bit here is that optional blocks sometimes reference other optional blocks that are *not* part of the same chain e.g. a(c?.d)?.d. See code + comments in `traverseOptionalBlock` for how we avoid concatenating unrelated blocks.
      
      2. Adding optional chains into non-null object calculation.
      (Note that marking `a?.b` as 'non-null' means that `a?.b.c` is safe to evaluate, *not* `(a?.b).c`. Happy to rename this / reword comments accordingly if there's a better term)
      This pass is split into two stages. (1) collecting non-null objects by block and (2) propagating non-null objects across blocks. The only significant change here was to (2). We add an extra reduce step `X=Reduce(Union(X, Intersect(X_neighbors)))` to merge optional and non-optional nodes (e.g. nonNulls=`{a, a?.b}` reduces to `{a, a.b}`)
      
      3. Adding optional chains into dependency calculation.
      This was the trickiest. We need to take the "maximal" property chain as a dependency. Prior to this PR, we avoided taking subpaths e.g. `a.b` of `a.b.c` as dependencies by only visiting non-PropertyLoad/LoadLocal instructions. This effectively only recorded the property-path at site-of-use.
      
          Unfortunately, this *quite* doesn't work for optional chains for a few reasons:
          - We would need to skip relevant `StoreLocal`/`Branch terminal` instructions (but only those within optional blocks that have been successfully read).
          - Given an optional chain, either (1) only a subpath or (2) the entire path can be represented as a PropertyLoad. We cannot directly add the last hoistable optional-block as a dependency as MethodCalls are an edge case e.g. given a?.b.c(), we should depend on `a?.b`, not `a?.b.c`
            This means that we add its dependency at either the innermost unhoistable optional-block or when encountering it within its phi-join.
      
      4. Handle optional chains in DeriveMinimalDependenciesHIR.
      This was also a bit tricky to formulate. Ideally, we would avoid a 2^3 case join (cond | uncond cfg, optional | not optional load, access | dependency). This PR attempts to simplify by building two trees
          1. First add each hoistable path into a tree containing `Optional | NonOptional` nodes.
          2. Then add each dependency into another tree containing `Optional | NonOptional`, `Access | Dependency` nodes, truncating the dependency at the earliest non-hoistable node (i.e. non-matching pair when walking the hoistable tree)
      
      ghstack-source-id: a2170f26280dfbf65a4893d8a658f863a0fd0c88
      Pull Request resolved: https://github.com/facebook/react/pull/31037
      0751fac7
  4. Oct 02, 2024
    • Timothy Yung's avatar
      Define `HostInstance` type for React Native (#31101) · 459fd418
      Timothy Yung authored
      ## Summary
      
      Creates a new `HostInstance` type for React Native, to more accurately
      capture the intent most developers have when using the `NativeMethods`
      type or `React.ElementRef<HostComponent<T>>`.
      
      Since `React.ElementRef<HostComponent<T>>` is typed as
      `React.AbstractComponent<T, NativeMethods>`, that means
      `React.ElementRef<HostComponent<T>>` is equivalent to `NativeMethods`
      which is equivalent to `HostInstance`.
      
      
      ## How did you test this change?
      
      ```
      $ yarn
      $ yarn flow fabric
      ```
      459fd418
    • Sebastian Markbåge's avatar
      [Flight] Allow aborting encodeReply (#31106) · 99c056ab
      Sebastian Markbåge authored
      Allow aborting encoding arguments to a Server Action if a Promise
      doesn't resolve. That way at least part of the arguments can be used on
      the receiving side. This leaves it unresolved in the stream rather than
      encoding an error.
      
      This should error on the receiving side when the stream closes but it
      doesn't right now in the Edge/Browser versions because closing happens
      immediately before we've had a chance to call `.then()` so the Chunks
      are still in pending state. This is an existing bug also in
      FlightClient.
      99c056ab
  5. Oct 01, 2024
    • Jack Pope's avatar
      Disable infinite render loop detection (#31088) · d8c90fa4
      Jack Pope authored
      We're seeing issues with this feature internally including bugs with
      sibling prerendering and errors that are difficult for developers to
      action on. We'll turn off the feature for the time being until we can
      improve the stability and ergonomics.
      
      This PR does two things:
      - Turn off `enableInfiniteLoopDetection` everywhere while leaving it as
      a variant on www so we can do further experimentation.
      - Revert https://github.com/facebook/react/pull/31061 which was a
      temporary change for debugging. This brings the feature back to
      baseline.
      d8c90fa4
    • Ruslan Lesiutin's avatar
      chore[react-devtools]: drop legacy context tests (#31059) · 6e612587
      Ruslan Lesiutin authored
      We've dropped the support for detecting changes in legacy Contexts in
      https://github.com/facebook/react/pull/30896.
      6e612587
    • Ruslan Lesiutin's avatar
      chore[react-devtools]: add legacy mode error message to the ignore list for tests (#31060) · 9ea5ffa9
      Ruslan Lesiutin authored
      Without this, the console gets spammy whenever we run React DevTools
      tests against React 18.x, where this deprecation message was added.
      9ea5ffa9
    • Ruslan Lesiutin's avatar
      fix[react-devtools]: request hook initialization inside http server response (#31102) · 40357fe6
      Ruslan Lesiutin authored
      Fixes https://github.com/facebook/react/issues/31100.
      
      There are 2 things:
      1. In https://github.com/facebook/react/pull/30987, we've introduced a
      breaking change: importing `react-devtools-core` is no longer enough for
      installing React DevTools global Hook. You need to call `initialize`, in
      which you may provide initial settings. I am not adding settings here,
      because it is not implemented, and there are no plans for supporting
      this.
      2. Calling `installHook` is not necessary inside `standalone.js`,
      because this script is running inside Electron wrapper (which is just a
      UI, not the app that we are debugging). We will loose the ability to use
      React DevTools on this React application, but I guess thats fine.
      40357fe6
    • Sebastian Markbåge's avatar
      [Flight] Serialize Server Components Props in DEV (#31105) · 654e387d
      Sebastian Markbåge authored
      This allows us to show props in React DevTools when inspecting a Server
      Component.
      
      I currently drastically limit the object depth that's serialized since
      this is very implicit and you can have heavy objects on the server.
      
      We previously was using the general outlineModel to outline
      ReactComponentInfo but we weren't consistently using it everywhere which
      could cause some bugs with the parsing when it got deduped on the
      client. It also lead to the weird feature detect of `isReactComponent`.
      It also meant that this serialization was using the plain serialization
      instead of `renderConsoleValue` which means we couldn't safely serialize
      arbitrary debug info that isn't serializable there.
      
      So the main change here is to call `outlineComponentInfo` and have that
      always write every "Server Component" instance as outlined and in a way
      that lets its props be serialized using `renderConsoleValue`.
      
      <img width="1150" alt="Screenshot 2024-10-01 at 1 25 05 AM"
      src="https://github.com/user-attachments/assets/f6e7811d-51a3-46b9-bbe0-1b8276849ed4">
      654e387d
    • Sebastian Markbåge's avatar
      [Flight] Serialize Error Values (#31104) · 326832a5
      Sebastian Markbåge authored
      The idea is that the RSC protocol is a superset of Structured Clone.
      #25687 One exception that we left out was serializing Error objects as
      values. We serialize "throws" or "rejections" as Error (regardless of
      their type) but not Error values.
      
      This fixes that by serializing `Error` objects. We don't include digest
      in this case since we don't call `onError` and it's not really expected
      that you'd log it on the server with some way to look it up.
      
      In general this is not super useful outside throws. Especially since we
      hide their values in prod. However, there is one case where it is quite
      useful. When you replay console logs in DEV you might often log an Error
      object within the scope of a Server Component. E.g. the default RSC
      error handling just console.error and error object.
      
      Before this would just be an empty object due to our lax console log
      serialization:
      <img width="1355" alt="Screenshot 2024-09-30 at 2 24 03 PM"
      src="https://github.com/user-attachments/assets/694b3fd3-f95f-4863-9321-bcea3f5c5db4">
      After:
      <img width="1348" alt="Screenshot 2024-09-30 at 2 36 48 PM"
      src="https://github.com/user-attachments/assets/834b129d-220d-43a2-a2f4-2eb06921747d">
      
      TODO for a follow up: Flight Reply direction. This direction doesn't
      actually serialize thrown errors because they always reject the
      serialization.
      326832a5
    • Mofei Zhang's avatar
      [compiler] Renames and no-op refactor for next PR · c67e241c
      Mofei Zhang authored
      Rename for clarity:
      - `CollectHoistablePropertyLoads:Tree` -> `CollectHoistablePropertyLoads:PropertyPathRegistry`
          - `getPropertyLoadNode` -> `getOrCreateProperty`
          - `getOrCreateRoot` -> `getOrCreateIdentifier`
      - `PropertyLoadNode` -> `PropertyPathNode`
      
      Refactor to CFG joining logic for `CollectHoistablePropertyLoads`. We now write to the same set of inferredNonNullObjects when traversing from entry and exit blocks. This is more correct, as non-nulls inferred from a forward traversal should be included when computing the backward traversal (and vice versa). This fix is needed by an edge case in #31036
      
      Added invariant into fixed-point iteration to terminate (instead of infinite looping).
      
      ghstack-source-id: 1e8eb2d566b649ede93de9a9c13dad09b96416a5
      Pull Request resolved: https://github.com/facebook/react/pull/31036
      c67e241c
    • Mofei Zhang's avatar
      [compiler][fixtures] Patch error-handling edge case in snap evaluator · 2cbea245
      Mofei Zhang authored
      Fix edge case in which we incorrectly returned a cached exception instead of trying to rerender with new props.
      ghstack-source-id: 843fb85df4a2ae7a88f296104fb16b5f9a34c76e
      Pull Request resolved: https://github.com/facebook/react/pull/31082
      2cbea245
    • Mofei Zhang's avatar
      [compiler] repro for dep merging edge case (non-hir) · 5d12e9e1
      Mofei Zhang authored
      Found when writing #31037, summary copied from comments:
      
      This is an extreme edge case and not code we'd expect any reasonable developer to write. In most cases e.g. `(a?.b != null ? a.b : DEFAULT)`, we do want to take a dependency on `a?.b`.
      
      I found this trying to come up with edge cases that break the current dependency + CFG merging logic. I think it makes sense to error on the side of correctness. After all, we still take `a` as a dependency if users write `a != null ? a.b : DEFAULT`, and the same fix (understanding the `<hoistable> != null` test expression) works for both. Can be convinced otherwise though!
      
      ghstack-source-id: cc06afda59f7681e228495f5e35a596c20f875f5
      Pull Request resolved: https://github.com/facebook/react/pull/31035
      5d12e9e1
    • Mofei Zhang's avatar
      [compiler][hir-rewrite] Cleanup Identifier -> IdentifierId · 58a3ca3b
      Mofei Zhang authored
      Since removing ExitSSA, Identifier and IdentifierId should mean the same thing
      
      ghstack-source-id: 076cacbe8360e716b0555088043502823f9ee72e
      Pull Request resolved: https://github.com/facebook/react/pull/31034
      58a3ca3b
    • Mofei Zhang's avatar
      [compiler][hir-rewrite] Infer non-null props, destructure source · 8c89fa76
      Mofei Zhang authored
      Followup from #30894.
      This adds a new flagged mode `enablePropagateScopeDepsInHIR: "enabled_with_optimizations"`, under which we infer more hoistable loads:
      - it's always safe to evaluate loads from `props` (i.e. first parameter of a `component`)
      - destructuring sources are safe to evaluate loads from (e.g. given `{x} = obj`, we infer that it's safe to evaluate obj.y)
      - computed load sources are safe to evaluate loads from (e.g. given `arr[0]`, we can infer that it's safe to evaluate arr.length)
      
      ghstack-source-id: 32f3bb72e9f85922825579bd785d636f4ccf724d
      Pull Request resolved: https://github.com/facebook/react/pull/31033
      8c89fa76
    • Mofei Zhang's avatar
      [compiler][test fixtures] Add enablePropagateDepsInHIR to forked tests · 1a779207
      Mofei Zhang authored
      Annotates fixtures added in #31030 with `@enablePropagateDepsInHIR` to fork behavior (and commit snapshot differences)
      ghstack-source-id: e423e8c42db62f1bb87562b770761be09fc8ffc6
      Pull Request resolved: https://github.com/facebook/react/pull/31031
      1a779207
    • Mofei Zhang's avatar
      [compiler][test fixtures] Fork more fixtures for hir-rewrite · 943e45e9
      Mofei Zhang authored
      Followup from #30894 , not sure how these got missed. Note that this PR just copies the fixtures without adding `@enablePropagateDepsInHIR`. #31032 follows and actually enables the HIR-version of propagateScopeDeps to run. I split this out into two PRs to make snapshot differences easier to review, but also happy to merge
      
      Fixtures found from locally setting snap test runner to default to `enablePropagateDepsInHIR: 'enabled_baseline'` and forking fixtures files with different output.
      
      ghstack-source-id: 7d7cf41aa923d83ad49f89079171b0411923ce6b
      Pull Request resolved: https://github.com/facebook/react/pull/31030
      943e45e9
    • Ruslan Lesiutin's avatar
      fix[scripts/devtools/publish-release]: parse version list instead of handling 404 (#31087) · 2d16326d
      Ruslan Lesiutin authored
      Discovered yesterday while was publishing a new release.
      
      NPM `10.x.x` changed the text for 404 errors, so this check was failing.
      Instead of handling 404 as a signal, I think its better to just parse
      the whole list of versions and check if the new one is already there.
      2d16326d
  6. Sep 28, 2024
    • Lauren Tan's avatar
      [playground] Decouple playground from compiler · db240980
      Lauren Tan authored
      Currently the playground is setup as a linked workspace for the
      compiler which complicates our yarn workspace setup and means that snap
      can sometimes pull in a different version of react than was otherwise
      specified.
      
      There's no real reason to have these workspaces combined so let's split
      them up.
      
      ghstack-source-id: 56ab064b2fc45366f5d96d37c5d4c5dc26590234
      Pull Request resolved: https://github.com/facebook/react/pull/31081
      db240980
  7. Sep 27, 2024
  8. Sep 26, 2024