[pull] main from facebook:main #856
Open
Conversation
This file contains 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
![pull[bot]](https://avatars.githubusercontent.com/in/12910?s=60&v=4){kind=small}
Stacked on #30981. Same as #30967 but for effects. This logs a tree of components using `performance.measure()`. In addition to the previous render phase this logs one tree for each commit phase: - Mutation Phase - Layout Effect - Passive Unmounts - Passive Mounts I currently skip the Before Mutation phase since the snapshots are so unusual it's not worth creating trees for those. The mechanism is that I reuse the timings we track for `enableProfilerCommitHooks`. I track first and last effect timestamp within each component subtree. Then on the way up do we log the entry. This means that we don't include overhead to find our way down to a component and that we don't need to add any additional overhead by reading timestamps. To ensure that the entries get ordered correctly we need to ensure that the start time of each parent is slightly before the inner one.
Stacked on #30983. This colors each component entry by its self time from light to dark depending on how long it took. If it took longer than a cut off we color it red (the error color). <img width="435" alt="Screenshot 2024-09-16 at 11 48 15 PM" src="https://github.com/user-attachments/assets/5d0bda83-6205-40e9-bec1-b81db2d48b2d">
This adds an `InlineJsxTransform` optimization pass, toggled by the `enableInlineJsxTransform` flag. When enabled, JSX will be transformed into React Element object literals, preventing runtime overhead during element creation. TODO: - [ ] Add conditionals to make transform PROD-only - [ ] Make the React element symbol configurable so this works with runtimes that support `react.element` or `react.transitional.element` - [ ] Look into additional optimization to pass props spread through directly if none of the properties are mutated
Right now we are patching console 2 times: when hook is installed (before page is loaded) and when backend is connected. Because of this, even if user had `appendComponentStack` setting enabled, all emitted error and warning logs are not going to have component stacks appended. They also won't have component stacks appended retroactively when user opens browser DevTools (this is when frontend is initialized and connects to backend). This behavior adds potential race conditions with LogBox in React Native, and also unpredictable to the user, because in order to get component stacks logged you have to open browser DevTools, but by the time you do it, error or warning log was already emitted. To solve this, we are going to only patch console in the hook object, because it is guaranteed to load even before React. Settings are going to be synchronized with the hook via Bridge, and React DevTools Backend Host (React Native or browser extension shell) will be responsible for persisting these settings across the session, this is going to be implemented in a separate PR.
…gs (#30566) Stacked on #30564. We are no longer using browser theme in our console patching, this was removed in unification of console patching for strict mode, we started using ansi escape symbols and forking based on browser theme is no longer required - #29869 The real browser theme initialization for frontend is happening at the other place and is not affected: https://github.com/facebook/react/blob/40be968257a7a10a267210670103f20dd0429ef3/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js#L117-L120
Stacked on #30566 and whats under it. See [this commit](374fd73). It is mostly copying code from one place to another and updating tests. With these changes, for every console method that we patch, there is going to be a single applied patch: - For `error`, `warn`, and `trace` we are patching when hook is installed. This guarantees that component stacks are going to be appended even if browser DevTools are not opened. We pay some price for it, though: if user has browser DevTools closed and if at this point some warning or error is emitted (logged), the next time user opens browser DevTools, they are going to see `hook.js` as the source frame. Unfortunately, ignore listing from source maps is not applied retroactively, and I don't know if its a bug or just a design limitations. Once browser DevTools are opened, source maps will be loaded and ignore listing will be applied for all emitted logs in the future. - For `log`, `info`, `group`, `groupCollapsed` we are only patching when React notifies React DevTools about running in StrictMode. We unpatch the methods right after it.
…to frontend (#30610) Stacked on #30597 and whats under it. See [this commit](59b4efa). With this change, the initial values for console patching settings are propagated from hook (which is the source of truth now, because of #30596) to the UI. Instead of reading from `localStorage` the frontend is now requesting it from the hook. This happens when settings modal is rendered, and wrapped in a transition. Also, this is happening even if settings modal is not opened yet, so we have enough time to fetch this data without displaying loader or similar UI.
…s across sessions (#30636) Stacked on #30610 and whats under it. See [last commit](248ddba). Now, we are using [`chrome.storage`](https://developer.chrome.com/docs/extensions/reference/api/storage) to persist settings for the browser extension across different sessions. Once settings are updated from the UI, the `Store` will emit `settingsUpdated` event, and we are going to persist them via `chrome.storage.local.set` in `main/index.js`. When hook is being injected, we are going to pass a `Promise`, which is going to be resolved after the settings are read from the storage via `chrome.storage.local.get` in `hookSettingsInjector.js`.
Rewrite `containerInfo?.ownerDocument?.defaultView ?? window` to instead use a ternary. This changes the compilation output (see [bundle changes from #30951](d65fb06)). ```js // compilation of containerInfo?.ownerDocument?.defaultView ?? window var $jscomp$optchain$tmpm1756096108$1, $jscomp$nullish$tmp0; containerInfo = null != ($jscomp$nullish$tmp0 = null == containerInfo ? void 0 : null == ($jscomp$optchain$tmpm1756096108$1 = containerInfo.ownerDocument) ? void 0 : $jscomp$optchain$tmpm1756096108$1.defaultView) ? $jscomp$nullish$tmp0 : window; // compilation of ternary expression containerInfo = null != containerInfo && null != containerInfo.ownerDocument && null != containerInfo.ownerDocument.defaultView ? containerInfo.ownerDocument.defaultView : window; ``` This also reduces the number of no-op bundle syncs for Meta. Note that Closure compiler's `jscomp$optchain$tmp<HASH>` identifiers change when we rebuild (likely due to version number changes). See [workflow](https://github.com/facebook/react/actions/runs/10891164281/job/30221518374) for a PR that was synced despite making no changes to the runtime.
## Summary Builds `react-dom` for React Native so that it also populates the `builds/facebook-fbsource` branch. **NOTE:** For Meta employees, D61354219 is the internal integration. ## How did you test this change? ``` $ yarn build … $ ls build/facebook-react-native/react-dom/cjs ReactDOM-dev.js ReactDOM-prod.js ReactDOM-profiling.js ```
…ore/backend (#30987) Stacked on #30986. Previously, we would call `installHook` at a top level of the JavaScript module. Because of this, having `require` statement for `react-devtools-core` package was enough to initialize the React DevTools global hook on the `window`. Now, the Hook can actually receive an argument - initial user settings for console patching. We expose this as a function `initialize`, which can be used by third parties (including React Native) to provide the persisted settings. The README was also updated to reflect the changes.
…0995) If JSX receives a props spread without additional attributes (besides `ref` and `key`), we can pass the spread object as a property directly to avoid the extra object copy. ``` <Test {...propsToSpread} /> // {props: propsToSpread} <Test {...propsToSpread} a="z" /> // {props: {...propsToSpread, a: "z"}} ```
…sx (#30996) Based on #30995 ([rendered diff](https://github.com/jackpope/react/compare/inline-jsx-2...jackpope:react:inline-jsx-3?expand=1)) ____ Some apps still use `react.element` symbols. Not only do we want to test there but we also want to be able to upgrade those sites to `react.transitional.element` without blocking on the compiler (we can change the symbol feature flag and compiler config at the same time). The compiler runtime uses `react.transitional.element`, so the snap fixture will fail if we change the default here. However I confirmed that commenting out the fixture entrypoint and running snap with `react.element` will update the fixture symbols as expected.
In #30596 we've moved console patching to the global hook. Generally speaking, the patching happens even before React is loaded on the page. If browser DevTools were opened after when `console.error` or `console.warn` were called, the source script will be `hook.js`, because of the patching.  This is because ignore listing is not applied retroactively by Chrome DevTools. If you had it open before console calls, Hook script would be correctly filtered out from the stack:  I had hopes that the fix for https://issues.chromium.org/issues/345248263 will also apply ignore listing retroactively, but looks like we need to open a separate feature request for the Chrome DevTools team. With these changes, if user attempts to open `hook.js` script, they are going to see this message: 
## Overview Adds a lint rule to prevent optional chaining to catch issues like #30982 until we support optional chaining without a bundle impact.
Commit artifact actions are breaking after #30711 See: https://github.com/facebook/react/actions/runs/10930658977/job/30344033974 > mv: cannot stat 'build/facebook-react-native/react/dom/': No such file or directory After build, the new artifacts are in `/react-dom/cjs`, not `/react/dom/` ``` $> yarn build $> ls build/facebook-react-native/react/ # ... no dom $> ls build/facebook-react-native/react-dom/cjs ```
When aborting we currently don't produce a componentStack when aborting the shell. This is likely just an oversight and this change updates this behavior to be consistent with what we do when there is a boundary
) This tracks the current window.event.timeStamp the first time we setState or call startTransition. For either the blocking track or transition track. We can use this to show how long we were blocked by other events or overhead from when the user interacted until we got called into React. Then we track the time we start awaiting a Promise returned from startTransition. We can use this track how long we waited on an Action to complete before setState was called. Then finally we track when setState was called so we can track how long we were blocked by other word before we could actually start rendering. For a Transition this might be blocked by Blocking React render work. We only log these once a subsequent render actually happened. If no render was actually scheduled, then we don't log these. E.g. if an isomorphic Action doesn't call startTransition there's no render so we don't log it. We only log the first event/update/transition even if multiple are batched into it later. If multiple Actions are entangled they're all treated as one until an update happens. If no update happens and all entangled actions finish, we clear the transition so that the next time a new sequence starts we can log it. We also clamp these (start the track later) if they were scheduled within a render/commit. Since we share a single track we don't want to create overlapping tracks. The purpose of this is not to show every event/action that happens but to show a prelude to how long we were blocked before a render started. So you can follow the first event to commit. <img width="674" alt="Screenshot 2024-09-20 at 1 59 58 AM" src="https://github.com/user-attachments/assets/151ba9e8-6b3c-4fa1-9f8d-e3602745eeb7"> I still need to add the rendering/suspended phases to the timeline which why this screenshot has a gap. <img width="993" alt="Screenshot 2024-09-20 at 12 50 27 AM" src="https://github.com/user-attachments/assets/155b6675-b78a-4a22-a32b-212c15051074"> In this case it's a Form Action which started a render into the form which then suspended on the action. The action then caused a refresh, which interrupts with its own update that's blocked before rendering. Suspended roots like this is interesting because we could in theory start working on a different root in the meantime which makes this timeline less linear.
<!-- Thanks for submitting a pull request! We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory. Before submitting a pull request, please make sure the following is done: 1. Fork [the repository](https://github.com/facebook/react) and create your branch from `main`. 2. Run `yarn` in the repository root. 3. If you've fixed a bug or added code that should be tested, add tests! 4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development. 5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`. 6. If you need a debugger, run `yarn test --debug --watch TestName`, open `chrome://inspect`, and press "Inspect". 7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`). 8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files. 9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`). 10. If you haven't already, complete the CLA. Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html --> ## Summary Profiling fails sometimes because `onProfilingStatus` is called repeatedly on some occasions, e.g. multiple calls to `getProfilingStatus`. Subsequent calls should be a no-op if the profiling status hasn't changed. Reported via #30661 #28838. > [!TIP] > Hide whitespace changes on this PR <img width="328" alt="screenshot showing the UI controls for hiding whitespace changes on GitHub" src="https://github.com/user-attachments/assets/036385cf-2610-4e69-a717-17c05d7ef047"> ## How did you test this change? <!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. How exactly did you verify that your PR solves the issue you wanted to solve? If you leave this empty, your PR will very likely be closed. --> Tested as part of Fusebox implementation of reload-to-profile. #31021
A slight behavior change here too is that I now mark the start of the commit phase before the BeforeMutationEffect phase. This affects `<Profiler>` too. The named sequences are as follows: Render -> Suspended or Throttled -> Commit -> Waiting for Paint -> Remaining Effects The Suspended phase is only logged if we delay the Commit due to CSS / images. The Throttled phase is only logged if we delay the commit due to the Suspense throttling timer. <img width="1246" alt="Screenshot 2024-09-20 at 9 14 23 PM" src="https://github.com/user-attachments/assets/8d01f444-bb85-472b-9b42-6157d92c81b4"> I don't yet log render phases that don't complete. I think I also need to special case renders that or don't commit after being suspended.
The trailing / was being omitted, so instead of moving the cjs directory itself, it would move only its contents instead. This broke some internal path assumptions. Additionally, updates the step to create the react-dom directory prior to moving. ghstack-source-id: b6eedb0c88cd3aa3a786a3d3d280ede5ee81a063 Pull Request resolved: #31026
Sometimes it is useful to bypass the revision check when we need to make changes to the runtime_commit_artifacts script. The `force` input can be passed via the GitHub UI for manual runs of the workflow. ghstack-source-id: cf9e32c01a565d86980277115f41e3e116adf376 Pull Request resolved: #31027
I messed up the yml syntax and also realized that our script doesn't currently handle renames or deletes, so I fixed that ghstack-source-id: 7d481a951abaabd1a2985c8959d8acb7103ed12e Pull Request resolved: #31028
This is a follow-up from #30528 to not only handle props (the critical change), but also the owner ~and stack~ of a referenced element. ~Handling stacks here is rather academic because the Flight Server currently does not deduplicate owner stacks. And if they are really identical, we should probably just dedupe the whole element.~ EDIT: Removed from the PR. Handling owner objects on the other hand is an actual requirement as reported in vercel/next.js#69545. This problem only affects the stable release channel, as the absence of owner stacks allows for the specific kind of shared owner deduping as demonstrated in the unit test.
…ng raw fiber instance (#31009) Related - #30899. Looks like this was missed. We actually do this when we record errors and warnings before sending them via Bridge: https://github.com/facebook/react/blob/e4953922a99b5477c3bcf98cdaa2b13ac0a81f0d/packages/react-devtools-shared/src/backend/fiber/renderer.js#L2169-L2173 So, what is happening in the end, errors or warnings are displayed in the Tree, but when user clicks on the component, nothing is shown, because `fiberToComponentLogsMap` has only `alternate` as a key.
#31010) Stacked on #31009. 1. Instead of keeping `showInlineWarningsAndErrors` in `Settings` context (which was removed in #30610), `Store` will now have a boolean flag, which controls if the UI should be displaying information about errors and warnings. 2. The errors and warnings counters in the Tree view are now counting only unique errors. This makes more sense, because it is part of the Elements Tree view, so ideally it should be showing number of components with errors and number of components of warnings. Consider this example: 2.1. Warning for element `A` was emitted once and warning for element `B` was emitted twice. 2.2. With previous implementation, we would show `3⚠️ `, because in total there were 3 warnings in total. If user tries to iterate through these, it will only take 2 steps to do the full cycle, because there are only 2 elements with warnings (with one having same warning, which was emitted twice). 2.3 With current implementation, we would show `2⚠️ `. Inspecting the element with doubled warning will still show the warning counter (2) before the warning message. With these changes, the feature correctly works. https://fburl.com/a7fw92m4
## Summary This fixes a typo in the error that gets reported when Float errors while hoisting a style tag that does not contain both `precedence` and `href`. There was a typo in _conflict_ and the last part of the sentence doesn't make sense. I assume it wasn't needed since the message already suggests moving the style tag to the head manually.
Also rename RootDidNotComplete to RootSuspendedAtTheShell since it specifically means something suspended in the shell during hydration.
This includes: - `Interrupted Render`: Interrupted Renders (setState or ping at higher priority) - `Prewarm`: Suspended Renders outside a Suspense boundary (RootSuspendedWithDelay/RootSuspendedAtTheShell) - `Errored Render`: Render that errored somewhere in the tree (Fatal or Not) (which may or may not be retried and then complete) - `Teared Render`: Due to useSyncExternalStore not matching (which will do another sync attempt) Suspended Commit: <img width="893" alt="Screenshot 2024-11-14 at 11 47 40 PM" src="https://github.com/user-attachments/assets/b25a6a8b-a5e9-4d66-b325-57aef4bf9dad"> Errored with a second recovery attempt that also errors: <img width="976" alt="Screenshot 2024-11-15 at 12 09 06 AM" src="https://github.com/user-attachments/assets/9ce52cbb-b587-4f1e-8b67-e51d9073ae5b">
We were previously filtering out `ref.current` dependencies in propagateScopeDependencies:checkValidDependency`. This is incorrect. Instead, we now always take a dependency on ref values (the outer box) as they may be reactive. Pruning is done in pruneNonReactiveDependencies. This PR includes a small patch to `collectReactiveIdentifier`. Prior to this, we conservatively assumed that pruned scopes always produced reactive declarations. This assumption fixed a bug with non-reactivity, but some of these declarations are `useRef` calls. Now we have special handling for this case ```js // This often produces a pruned scope React.useRef(1); ``` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31521). * #31202 * #31203 * #31201 * #31200 * __->__ #31521
…31200) Recursively visit inner function instructions to extract dependencies instead of using `LoweredFunction.dependencies` directly. This is currently gated by enableFunctionDependencyRewrite, which needs to be removed before we delete `LoweredFunction.dependencies` altogether (#31204). Some nice side effects - optional-chaining deps for inner functions - full DCE and outlining for inner functions (see #31202) - fewer extraneous instructions (see #31204) - --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31200). * #31202 * #31203 * #31201 * __->__ #31200 * #31521
`JSXMemberExpression` is currently the only instruction (that I know of) that directly references identifier lvalues without a corresponding `LoadLocal`. This has some side effects: - deadcode elimination and constant propagation now reach JSXMemberExpressions - we can delete `LoweredFunction.dependencies` without dangling references (previously, the only reference to JSXMemberExpression objects in HIR was in function dependencies) - JSXMemberExpression now is consistent with all other instructions (e.g. has a rvalue-producing LoadLocal) ' --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31201). * #31202 * #31203 * __->__ #31201 * #31200 * #31521
Add more `FIXTURE_ENTRYPOINT`s ' --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31203). * #31202 * __->__ #31203 * #31201 * #31200 * #31521
Now that we rely on function context exclusively, let's clean up `HIRFunction.context` after DCE. This PR is in preparation of #31204, which would otherwise have unnecessary declarations (of context values that become entirely DCE'd) ' --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31202). * __->__ #31202 * #31203 * #31201 * #31200 * #31521
Scaffolds the initial `useResourceEffect` dispatcher type. This will eventually be folded into `useEffect` et al as an overload. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31555). * #31523 * #31557 * #31556 * __->__ #31555
Adds a new feature flag for `enableUseResourceEffectHook`. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31556). * #31523 * #31557 * __->__ #31556 * #31555
Adds a new `Effect` type which for now just points to the `SimpleEffect` type, in prepartion for later in the stack where we add more. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31557). * #31523 * __->__ #31557 * #31556 * #31555
…seActionException sentinel (#31554) This lets us track separately if something was suspended on an Action using useActionState rather than suspended on Data. This approach feels quite bloated and it seems like we'd eventually might want to read more information about the Promise that suspended and the context it suspended in. As a more general reason for suspending. The way useActionState works in combination with the prewarming is quite unfortunate because 1) it renders blocking to update the isPending flag whether you use it or not 2) it prewarms and suspends the useActionState 3) then it does another third render to get back into the useActionState position again.
This PR introduces a new experimental hook `useResourceEffect`, which is something that we're doing some very early initial tests on. This may likely not pan out and will be removed or modified if so. Please do not rely on it as it will break.
Since `enableRefAsProp` shipped everywhere, the ReactElement implementation on prod puts refs on both `element.ref` and `element.props.ref`. Here we let the `ref` case fall through so its now available on props, matching the JSX runtime.
Repro for bug in our type inference system. We currently propagate inferred types through control flow / potential type guards. Note that this is inconsistent with both [Flow](https://flow.org/try/#1N4Igxg9gdgZglgcxALlAIwIZoKYBsD6uEEAztvhgE6UYCe+JADpdhgCYowa5kA0I2KAFcAtiRQAXSkOz9sADwxgJ+NPTbYuQ3BMnTZA+Y2yU4IwRO4A6SFBIrGVDGM7c+h46fNRLuKxJIGWh8MeT0ZfhYlCStpHzNsFBAMIQkIEQwJODAQfiEyfBE4eWw2fDgofDBMsAALfAA3KjgsXGxxZC4eAw0G-GhcWn9aY3wWZldu-g1mbGqJUoBaCRHEzrcDEgBrbAk62kXhXFxJ923d-cPRHEpTgyEoMDaqZdW7vKgoOfaSKgOKpqmDA+d4gB5fMA-P6LCCMLLQbiLOoYCqgh6-GDYRYIXYLSgkRZkCR4jpddwPfJLZjpOBkO4AX34kA0SRWxgABAAxYjsgC87OAAB0oOzReythU2Mh2YKQNyILLeMKxeymrgZNLhCIbsL6QBuYVs7DsgBCVD5AuVYolUClMpAZsoiqtorVGvZWpuSqg9OFMAeyjg0HZdTmW3lAAp5NKAPJoABWcwkAEppWZGLg4O12fJ2bSuTyhSKxSwJEJKCKAOQ2tiVvMi3MAMkbOasNb5vP5svlsoNPuFfoD8JFGQqUel8vZAB9TVReCHoHa0MRnlBUwWIJbi6K4DB2RHbGxk1uVSrd-uAIShsDh4hR5PHoun5-siS1SgQADuHuw34AotQECUBGsqysmfYvuyvrbqepblg2EFitBKpwRWOZ9vSuQgA0JgkEGUBJBk9gmCA9JAA) and [Typescript](https://www.typescriptlang.org/play/?#code/C4TwDgpgBAYg9nKBeKBvAUFLUDWBLAOwBMAuKAInjnIBpNsA3AQwBsBXCMgtgWwCMIAJ3QBfANzpQkKACEmg5GnpZ8xMuTmDayqM3aco3fkLoj0AMzYEAxsDxwCUawAsI1nFQAUADzJw+AFZuwACUZEwAzhFCwBFQ3lB4cVRK2InmUJ4AhJ4A5KpEuYmOCQBkpfEAdAXISCiUCOQhIalp2MDOgnAA7oYQvQCigl2CnuRWEN6QthBETTpmZhZWtvaOPEyEPmQpAD6y8jRODqRQfAgsEEwEYbAIrVh4GZ7WJy0Ybdgubh4IPiEST5YQQQYBsQQlQHYMxpEFgiHxCQiIA)
Fixes #31331 ## Summary There is a bug in playground(#31331) which doesnt support 'use memo' or 'use no memo' directives. Its misleading while debugging components in the playground ## How did you test this change? Ran test cases and added a few extra test cases as well ## Changes 1) Adds support for 'use memo' and 'use no memo' 2) Cleanup E2E test cases a bit 3) Adds test cases for use memo 4) Added documentation to run test cases ## Implementation `parseFunctions` returns a set of functions to be compiled. But, it doesnt filter out/handle memoized opted/un-opted functions using directives. ive just created a `compile` flag to enable/disable compiling [here](https://github.com/facebook/react/pull/31561/files#diff-305de47a3fe3ce778e22d5c5cf438419a59de8e7f785b45f659e7b41b1e30b03R113) Then I am just skipping those functions from getting compile [here](https://github.com/facebook/react/pull/31561/files#diff-305de47a3fe3ce778e22d5c5cf438419a59de8e7f785b45f659e7b41b1e30b03R253)
Small change to always upload test results from CI even if the test failed.
--- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31572). * #31573 * __->__ #31572
Our e2e setup with monaco is kinda brittle since it relies on the dom. It seems like longish text gets truncated so let's just simpify all these test cases. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31573). * __->__ #31573 * #31572
…n identifier with an `as` expression (#31119)
``` => Found "react@0.0.0-experimental-4beb1fd8-20241118" info Reasons this module exists - "_project_#babel-plugin-react-compiler" depends on it - Hoisted from "_project_#babel-plugin-react-compiler#react" - Hoisted from "_project_#snap#react" info Disk size without dependencies: "252KB" info Disk size with unique dependencies: "252KB" info Disk size with transitive dependencies: "252KB" info Number of shared dependencies: 0 ✨ Done in 0.60s. ``` ``` => Found "react-dom@0.0.0-experimental-4beb1fd8-20241118" info Reasons this module exists - "_project_#babel-plugin-react-compiler" depends on it - Hoisted from "_project_#babel-plugin-react-compiler#react-dom" - Hoisted from "_project_#snap#react-dom" info Disk size without dependencies: "8.04MB" info Disk size with unique dependencies: "8.17MB" info Disk size with transitive dependencies: "8.17MB" info Number of shared dependencies: 1 ✨ Done in 0.56s. ``` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31585). * #31586 * __->__ #31585
``` => Found "hermes-parser@0.25.1" info Reasons this module exists - "_project_#prettier-plugin-hermes-parser" depends on it - Hoisted from "_project_#prettier-plugin-hermes-parser#hermes-parser" - Hoisted from "_project_#eslint-plugin-react-compiler#hermes-parser" - Hoisted from "_project_#snap#hermes-parser" - Hoisted from "_project_#snap#babel-plugin-syntax-hermes-parser#hermes-parser" - Hoisted from "_project_#eslint-plugin-react-compiler#hermes-eslint#hermes-parser" info Disk size without dependencies: "1.49MB" info Disk size with unique dependencies: "1.82MB" info Disk size with transitive dependencies: "1.82MB" info Number of shared dependencies: 1 ✨ Done in 0.81s. ``` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/31586). * __->__ #31586 * #31585
…ded (#31552) When we suspend the render with delay, we won't do any more work until we get some kind of another update/ping. It's because conceptually something is suspended and then will update later. We need to highlight this period to show why it's not doing any work. We fill the empty space with "Suspended". This stops whenever the same lane group starts rendering again. Clamped by the preceeding start time/event time/update time. <img width="902" alt="Screenshot 2024-11-15 at 1 01 29 PM" src="https://github.com/user-attachments/assets/acf9dc9a-8fc3-4367-a8b0-d19f9c9eac73"> Ideally we would instead start the next render and suspend the work loop at all places we suspend. In that mode this will instead show up as a very long "Render" with a "Suspended" period instead highlighted in the Components track as one component is suspended. We'll soon have that for `use()` but not all updates so this covers the rest. One issue with `useActionState` is that it is implemented as suspending at the point of the `useActionState` which means that the period of the Action shows up as a suspended render instead of as an Action which happens for raw actions. This is not really how you conceptually think about it so we need some special case for `useActionState`. In the screenshot above, the first "Suspended" is actually awaiting an Action and the second "Suspended" is awaiting the data from it.
Stacked on #31552. Must be tested with `enableSiblingPrerendering` off since the `use()` optimization is not on there yet. This adds a span to the Components track when we yield in the middle of the event loop. In this scenario, the "Render" span continues through out the Scheduler track. So you can see that the Component itself might not take a long time but yielding inside of it might. This lets you see if something was blocking the React render loop while yielding. If we're blocked 1ms or longer we log that as "Blocked". If we're yielding due to suspending in the middle of the work loop we log this as "Suspended". <img width="837" alt="Screenshot 2024-11-16 at 1 15 14 PM" src="https://github.com/user-attachments/assets/45a858ea-17e6-416c-af1a-78c126e033f3"> If the render doesn't commit because it restarts due to some other prewarming or because some non-`use()` suspends, it doesn't have from context components. <img width="971" alt="Screenshot 2024-11-16 at 1 13 55 PM" src="https://github.com/user-attachments/assets/a67724f8-702e-4e7d-9499-9ffc09541a61"> The `useActionState` path doesn't work yet because the `use()` optimization doesn't work there for some reason. But the idea is that it should mark the time that the component is blocked as Action instead of Suspended.
Happens to the best of us.
Fixes a bug with the experimental `useResourceEffect` hook where we would compare the wrong deps when there happened to be another kind of effect preceding the ResourceEffect. To do this correctly we need to add a pointer to the ResourceEffect's identity on the update. I also unified the previously separate push effect impls for resource effects since they are always pushed together as a unit.
This avoid re-emitting the yellow "Event" log when we ping inside the original event. Instead of treating events as repeated when we get repeated updates, we treat them as repeated if we've ever logged out this event before. Additionally, in the case the prerender sibling flag is on we need to ensure that if a render gets interrupted when it has been suspended we treat that as "Prewarm" instead of "Interrupted Render". Before: <img width="539" alt="Screenshot 2024-11-19 at 2 39 44 PM" src="https://github.com/user-attachments/assets/190ca50c-5168-40d8-a6fd-6b9a583af1f0"> After: <img width="1004" alt="Screenshot 2024-11-21 at 4 53 16 PM" src="https://github.com/user-attachments/assets/0c441ada-1ed1-412c-8935-aaf040c25dfe">
This ensures that we mark the time from ping until we render as "Blocked". We intentionally don't want to show the event time even if it's something like "load" because it draws attention away from interactions etc. <img width="577" alt="Screenshot 2024-11-21 at 7 22 39 PM" src="https://github.com/user-attachments/assets/70cca2e8-bd5e-489f-98f0-b4dfee5940af">
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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]
Can you help keep this open source service alive? 💖 Please sponsor : )