[pull] main from facebook:main #854
Open
+370,237
−3,670
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}
This compresses more efficiently than the base64 encoding we were previously using, which makes sharing URLs a little less unwieldy and takes up less space in local storage. Using some real code as an example, lz-string compresses to 8040 bytes, whereas the original base64 encoding we were using compresses to 16504 bytes ghstack-source-id: b8f1089889b94b07d6f419606b798ffddb8863ba Pull Request resolved: facebook/react-forget#2834
ghstack-source-id: e8dba44d246d53f60a0858710cedf75c77c6c7e4 Pull Request resolved: facebook/react-forget#2835
ghstack-source-id: fc2d58e4d5195c3ec157f65b3e1c94de39ff2070 Pull Request resolved: facebook/react-forget#2836
It was never clear to me what the difference between Wipe and Reset was. Let's just get rid of one and reset to something more useful instead of fibonacci. ghstack-source-id: 4f88a1c1da2d0fd9e1f26e4859c12db0fc961af2 Pull Request resolved: facebook/react-forget#2837
ghstack-source-id: 9fd0c2f8d70f8782602b0e4f8535b6be3ed89782 Pull Request resolved: facebook/react-forget#2838
ghstack-source-id: 4fdf672f02144e550d50b17f396a5d6d82dab5bd Pull Request resolved: facebook/react-forget#2839
ghstack-source-id: 56e72c56782274bdac92fdd131d2c9b33eb19052 Pull Request resolved: facebook/react-forget#2840
ghstack-source-id: 1c23bbbb55c1582a03674897b57c5061b895be83 Pull Request resolved: facebook/react-forget#2841
ghstack-source-id: 3904ff9beea21ffbb79a6a15fcfbb72f04a2d170 Pull Request resolved: facebook/react-forget#2842
Fixes a tiny inconsistency with compiler options where one was all uppercase and one all lowercase by normalizing to lowercase regardless of the casing of the user's config. ghstack-source-id: fe60a3259de89a1b3fdd7475950e16e96cc57f6b Pull Request resolved: facebook/react-forget#2832
ghstack-source-id: f9e80fc0c928e1ce1ec698d4d952e11402054843 Pull Request resolved: facebook/react-forget#2833
ghstack-source-id: 1b52cb312a5f26e8e058bfe4a29b586ffaeb25d7 Pull Request resolved: facebook/react-forget#2843
ghstack-source-id: d646c84815c324bd490584d34488a73f5e39d589 Pull Request resolved: facebook/react-forget#2845
To make the polyfill work well with devtools, we add this sentinel. Devtools will look for this sentinel before adding the Forget badge. ghstack-source-id: d246040f67da48fa46f818753c8bf26dff56f390 Pull Request resolved: facebook/react-forget#2847
The compiler has an optimisation where it transforms a simple arrow function with only a return statement to a implicit arrow function. In the case, there's a directive in this simple arrow function, the directive gets dropped. Instead of dropping the directive, the compiler should perform this optimisation only if there are no directives. ghstack-source-id: 514cd2440025986a2d6d950694a7339d779b09f2 Pull Request resolved: facebook/react-forget#2848
No need to commit this in the repo, but keeping this locally helps with building the feedback repo. ghstack-source-id: 28f08992b1ab856567a5338af07e12ac820a7298 Pull Request resolved: facebook/react-forget#2854
This allows the plugin to be configured to run on an allowlist, rather than compiling all files helping with an incremental rollout plan. The sources option takes both an array of path strings or a function to be flexible. For now I've left this be optional but we can make it required. ghstack-source-id: 282a33dc8d08d47f699894692e0fcc813dff5b77 Pull Request resolved: facebook/react-forget#2855
ghstack-source-id: 693a3526a73f1fbd25f4e59416f8c65a0f8f1235 Pull Request resolved: facebook/react-forget#2857
ghstack-source-id: cbf62aec4e768249a8c7f44fe6f3852183127415 Pull Request resolved: facebook/react-forget#2858
ghstack-source-id: 0c362a349de86a07b4e9e381b942939ce4a24e69 Pull Request resolved: facebook/react-forget#2859
ghstack-source-id: 48ce4b55a8b4b5a22c8ec29f6787732ab2a67778 Pull Request resolved: facebook/react-forget#2860
ghstack-source-id: 1a88145049d7f7acb01748ad6ec4dd1500781766 Pull Request resolved: facebook/react-forget#2861
ghstack-source-id: a5623aa4f29e15c89d2dfbe855bab9843e240a8a Pull Request resolved: facebook/react-forget#2862
ghstack-source-id: 34e5507c08fb883c987c88f415158fac781a5f8e Pull Request resolved: facebook/react-forget#2863
ghstack-source-id: ff5bcaa7ab219035f57dc3dc3396c9a324896d4b Pull Request resolved: facebook/react-forget#2869
ghstack-source-id: dbc012018ecc73007dfb3e745615605512ab6867 Pull Request resolved: facebook/react-forget#2870
ghstack-source-id: a715b1385da6648d867118d2b7486ffdb0ff89f6 Pull Request resolved: facebook/react-forget#2871
ghstack-source-id: fcda8140310d6e1201df35f399020b22b6dccb08 Pull Request resolved: facebook/react-forget#2872
ghstack-source-id: c4f5777b85751e9d5f0323384160adfba55d883f Pull Request resolved: facebook/react-forget#2875
ghstack-source-id: f83e9e28c1cdf31dba718e62db12aaa3a5f9ddab Pull Request resolved: facebook/react-forget#2876
Fixes https://x.com/raibima/status/1794395807216738792 The issue is that if you pass a global-modifying function as prop to JSX, we currently report that it's invalid to modify a global during rendering. The problem is that we don't really know when/if the child component will actually call that function prop. It would be against the rules to call the function during render, but it's totally fine to call it during an event handler or from a useEffect. Since we don't know at the call-site how the child will use the function, we should allow such calls. In the future we could improve this in a few ways: * For all functions that modify globals, codegen an assertion or warning into the function that fires if it's called "during render". We'd have to precisely define what "during render" is, but this would at least help developers catch this dynamically. * Use the type system to distinguish "event/effect" and "render" functions to help developers avoid accidentally mutating globals during render. ghstack-source-id: 4aba4e6d214fd6c062e4029294efe9b8fe25cd83 Pull Request resolved: #29591
ghstack-source-id: 65dd14fe9b37328bd60fe791b23dde54da10b285 Pull Request resolved: #29175
#29650) ## Summary There are already most arithmetic operators in constant propagation: `+`, `-`, `*`, `/`. We could add more, namely: `|`, `&`, `^`, `<<`, `>>`, `>>>` and `%`: Input: ```js function f() { return [ 123.45 | 0, 123.45 & 0, 123.45 ^ 0, 123 << 0, 123 >> 0, 123 >>> 0, 123.45 | 1, 123.45 & 1, 123.45 ^ 1, 123 << 1, 123 >> 1, 123 >>> 1, 3 ** 2, 3 ** 2.5, 3.5 ** 2, 2 ** 3 ** 0.5, 4 % 2, 4 % 2.5, 4 % 3, 4.5 % 2, ]; } ``` Output: ```js function f() { return [ 123, 0, 123, 123, 123, 123, 123, 1, 122, 246, 61, 61, 9, 15.588457268119896, 12.25, 3.3219970854839125, 0, 1.5, 1, 0.5, ]; } ``` Resolves #29649 ## How did you test this change? See tests. Note: This PR was done without waiting for approval in #29649, so feel free to just close it without any comment.
## Summary This PR add tests for `ReactNativeAttributePayloadFabric.js`. It introduces `ReactNativeAttributePayloadFabric-test.internal.js`, which is a copy-paste of `ReactNativeAttributePayload-test.internal.js`. On top of that, there is a bunch of new test cases for the `ReactNativeAttributePayloadFabric.create` function. ## How did you test this change? ``` yarn test packages/react-native-renderer ```
## Summary #29088 introduced a regression triggering this warning when rendering flattened positional children: > Each child in a list should have a unique "key" prop. The specific scenario that triggers this is when rendering multiple positional children (which do not require unique `key` props) after flattening them with one of the `React.Children` utilities (e.g. `React.Children.toArray`). The refactored logic in `React.Children` incorrectly drops the `element._store.validated` property in `__DEV__`. This diff fixes the bug and introduces a unit test to prevent future regressions. ## How did you test this change? ``` $ yarn test ReactChildren-test.js ```
#29236 caused issues for internal syncs at Meta, because we were computing version numbers using file hashes (to eliminate "no-op" internal sync commits). The problem is that since version numbers may not be consistent across synced files (e.g. if some files have not changed in recent commits), the newly introduced version mismatch check fails. There's some more work that needs to be done here to restore the benefits of file-specific hashing, but for now this simply reverts the content hash changes from the following PRs: - #28633 (95319ab) - #28590 (37676ab) - #28582 (cb076b5) - #26734 (5dd90c5) - #26331 (3cad3a5)
…29652) Partially reverts #28593. While rolling out RDT 5.2.0, I've observed some issues on React Native side: hooks inspection for some complex hook trees, like in AnimatedView, were broken. After some debugging, I've noticed a difference between what is in frame's source. The difference is in the top-most frame, where with V8 it will correctly pick up the `Type` as `Proxy` in `hookStack`, but for Hermes it will be `Object`. This means that for React Native this top most frame is skipped, since sources are identical. Here I am reverting back to the previous logic, where we check each frame if its a part of the wrapper, but also updated `isReactWrapper` function to have an explicit case for `useFormStatus` support.
…ks (#29632) We have three kinds of stacks that we send in the RSC protocol: - The stack trace where a replayed `console.log` was called on the server. - The JSX callsite that created a Server Component which then later called another component. - The JSX callsite that created a Host or Client Component. These stack frames disappear in native stacks on the client since they're executed on the server. This evals a fake file which only has one call in it on the same line/column as the server. Then we call through these fake modules to "replay" the callstack. We then replay the `console.log` within this stack, or call `console.createTask` in this stack to recreate the stack. The main concern with this approach is the performance. It adds significant cost to create all these eval:ed functions but it should eventually balance out. This doesn't yet apply source maps to these. With source maps it'll be able to show the server source code when clicking the links. I don't love how these appear. - Because we haven't yet initialized the client module we don't have the name of the client component we're about to render yet which leads to the `<...>` task name. - The `(async)` suffix Chrome adds is still a problem. - The VMxxxx prefix is used to disambiguate which is noisy. Might be helped by source maps. - The continuation of the async stacks end up rooted somewhere in the bootstrapping of the app. This might be ok when the bootstrapping ends up ignore listed but it's kind of a problem that you can't clear the async stack. <img width="927" alt="Screenshot 2024-05-28 at 11 58 56 PM" src="https://github.com/facebook/react/assets/63648/1c9d32ce-e671-47c8-9d18-9fab3bffabd0"> <img width="431" alt="Screenshot 2024-05-28 at 11 58 07 PM" src="https://github.com/facebook/react/assets/63648/52f57518-bbed-400e-952d-6650835ac6b6"> <img width="327" alt="Screenshot 2024-05-28 at 11 58 31 PM" src="https://github.com/facebook/react/assets/63648/d311a639-79a1-457f-9a46-4f3298d07e65"> <img width="817" alt="Screenshot 2024-05-28 at 11 59 12 PM" src="https://github.com/facebook/react/assets/63648/3aefd356-acf4-4daa-bdbf-b8c8345f6d4b">
Follow up to #29632. It's possible for `eval` to throw such as if we're in a CSP environment. This is non-essential debug information. We can still proceed to create a fake stack entry. It'll still have the right name. It just won't have the right line/col number nor source url/source map. It might also be ignored listed since it's inside Flight.
Like mobx, this library depends on mutating a Proxied store and breaks reference equality checks.
## Summary In #29088, the validation logic for `React.Children` inspected whether `mappedChild` — the return value of the map callback — has a valid `key`. However, this deviates from existing behavior which only warns if the original `child` is missing a required `key`. This fixes false positive `key` validation warnings when using `React.Children`, by validating the original `child` instead of `mappedChild`. This is a more general fix that expands upon my previous fix in #29662. ## How did you test this change? ``` $ yarn test ReactChildren-test.js ```
When defining a displayName on forwardRef/memo we forward that name to the inner function. We used to use displayName for this but in #29206 I switched this to use `"name"`. That's because V8 doesn't use displayName, it only uses the overridden name in stack traces. This is the only thing covered by our tests for component stacks. However, I realized that Safari only uses displayName and not the name. So this sets both.
We're getting a ton of issues filed using the blank template, for example these airline support tickets: https://github.com/facebook/react/issues/29678 I think someone somewhere is linking to our issues with pre-filled content. This fixes it by forcing a template to be used.
This lets any element created from the server, to bottom out with a client "owner" which is the creator of the Flight request. This could be a Server Action being invoked or a router. This is similar to how a client element bottoms out in the creator of the root element without an owner. E.g. where the root app element was created. Without this, we inherit the task of whatever is currently executing when we're parsing which can be misleading. Before: <img width="507" alt="Screenshot 2024-05-30 at 12 06 57 PM" src="https://github.com/facebook/react/assets/63648/e234db7e-67f7-404c-958a-5c5500ffdf1f"> After: <img width="555" alt="Screenshot 2024-05-30 at 4 59 04 PM" src="https://github.com/facebook/react/assets/63648/8ba6acb4-2ffd-49d4-bd44-08228ad4200e"> The before/after doesn't show much of a difference here but that's just because our Flight parsing loop is an async, which maybe it shouldn't be because it can be unnecessarily deep, and it creates a hidden line for every loop. That's what the `Promise.then` is. If the element is lazily initialized it's worse because we can end up in an unrelated render task as the owner - although that's its own problem.
Summary: This adds a compiler option to not drop existing manual memoization and leaving useMemo/useCallback in the generated source. Why do we need this, given that we also have options to validate or ensure that existing memoization is preserved? It's because later diffs on this stack are designed to alter the behavior of the memoization that the compiler emits, in order to detect rules of react violations and debug issues. We don't want to change the behavior of user-level memoization, however, since doing so would be altering the semantics of the user's program in an unacceptable way. ghstack-source-id: 89dccdec9ccb4306b16e849e9fa2170bb5dd021f Pull Request resolved: #29654
Summary: This adds a debugging mode to the compiler that simply adds a `|| true` to the guard on all memoization blocks, which results in the generated code never using memoized values and always recomputing them. This is designed as a validation tool for the compiler's correctness--every program *should* behave exactly the same with this option enabled as it would with it disabled, and so any difference in behavior should be investigated as either a compiler bug or a pipeline issue. (We add `|| true` rather than dropping the conditional block entirely because we still want to exercise the guard tests, in case the guards themselves are the source of an error, like reading a property from undefined in a guard.) ghstack-source-id: 955a47ec1689842da82552225a19a1008c57fe2c Pull Request resolved: #29655
…zation Summary: The essential assumption of the compiler is that if the inputs to a computation have not changed, then the output should not change either--computation that the compiler optimizes is idempotent. This is, of course, known to be false in practice, because this property rests on requirements (the Rules of React) that are loosely enforced at best. When rolling out the compiler to a codebase that might have rules of react violations, how should developers debug any issues that arise? This diff attempts one approach to that: when the option is set, rather than simply skipping computation when dependencies haven't changed, we will *still perform the computation*, but will then use a runtime function to compare the original value and the resultant value. The runtime function can be customized, but the idea is that it will perform a structural equality check on the values, and if the values aren't structurally equal, we can report an error, including information about what file and what variable was to blame. This assists in debugging by narrowing down what specific computation is responsible for a difference in behavior between the uncompiled code and the program after compilation. ghstack-source-id: 50dad3dacfc7fef74be350431aa2ebf5e9cb0031 Pull Request resolved: #29656
Summary: jmbrown215 recently had an observation that the arguments to useState/useRef are only used when a component renders for the first time, and never afterwards. We can skip more computation that we previously could, with reactive blocks that previously recomputed values when inputs changed now only ever computing them on the first render. ghstack-source-id: 5d044ef787a7da901c70990f4399aa90c9b96802 Pull Request resolved: #29653
Summary: This PR expands the analysis from the previous in the stack in order to also capture when a value can incorrectly change within a single render, rather than just changing between two renders. In the case where dependencies have changed and so a new value is being computed, we now compute the value twice and compare the results. This would, for example, catch when we call Math.random() in render. The generated code is a little convoluted, because we don't want to have to traverse the generated code and substitute variable names with new ones. Instead, we save the initial value to the cache as normal, then run the computation block again and compare the resulting values to the cached ones. Then, to make sure that the cached values are identical to the computed ones, we reassign the cached values into the output variables. ghstack-source-id: d0f11a4cb2a612cbffdfdcaa9e75efbd6e38019f Pull Request resolved: #29657
…ive scopes for debugging Summary: Using the change detection code to debug codebases that violate the rules of react is a lot easier when we have a source location corresponding to the value that has changed inappropriately. I didn't see an easy way to track that information in the existing data structures at the point of codegen, so this PR adds locations to identifiers and reactive scopes (the location of a reactive scope is the range of the locations of its included identifiers). I'm interested if there's a better way to do this that I missed! ghstack-source-id: aed5f7eddae7256f41da4389e8f16fcb3daaee49 Pull Request resolved: #29658
When a component suspends with `use`, we switch to the "re-render" dispatcher during the subsequent render attempt, so that we can reuse the work from the initial attempt. However, once we run out of hooks from the previous attempt, we should switch back to the regular "update" dispatcher. This is conceptually the same fix as the one introduced in #26232. That fix only accounted for initial mount, but the useTransition regression test added in f829733 illustrates that we need to handle updates, too. The issue affects more than just useTransition but because most of the behavior between the "re-render" and "update" dispatchers is the same it's hard to contrive other scenarios in a test, which is probably why it took so long for someone to notice. Closes #28923 and #29209 --------- Co-authored-by: eps1lon <sebastian.silbermann@vercel.com>
Eslint rules should never throw, so if we fail to parse with Babel or Hermes, we should just ignore the error. This should fix issues such as trying to run the eslint rule on non tsx|ts|jsx|js files, Hermes parser not supporting certain JS syntax, etc. I didn't add a test for this as our eslint-rule-tester config uses hermes-eslint parser, so it wasn't possible to add a top level await as it would crash hermes-eslint before our rule was triggered. Similarly I couldn't add a test for non-JS files as it would not be parseable by hermes-eslint. Fixes #29107 ghstack-source-id: 60afcdb89ab4a8d2e4697cc50c5490803e7cbeac Pull Request resolved: #29631
…ce maps from (#29708) This lets you click a stack frame on the client and see the Server source code inline. <img width="871" alt="Screenshot 2024-06-01 at 11 44 24 PM" src="https://github.com/facebook/react/assets/63648/581281ce-0dce-40c0-a084-4a6d53ba1682"> <img width="840" alt="Screenshot 2024-06-01 at 11 43 37 PM" src="https://github.com/facebook/react/assets/63648/00dc77af-07c1-4389-9ae0-cf1f45199efb"> We could do some logic on the server that sends a source map url for every stack frame in the RSC payload. That would make the client potentially config free. However regardless we need the config to describe what url scheme to use since that’s not built in to the bundler config. In practice you likely have a common pattern for your source maps so no need to send data over and over when we can just have a simple function configured on the client. The server must return a source map, even if the file is not actually compiled since the fake file is still compiled. The source mapping strategy can be one of two models depending on if the server’s stack traces (`new Error().stack`) are source mapped back to the original (`—enable-source-maps`) or represents the location in compiled code (like in the browser). If it represents the location in compiled code it’s actually easier. You just serve the source map generated for that file by the tooling. If it is already source mapped it has to generate a source map where everything points to the same location (as if not compiled) ideally with a segment per logical ast node.
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 : )