Revert to 8310854ce #26638
Closed
Revert to 8310854ce #26638
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
…#26522) ## Summary Fixes facebook#24781 Restricting from editing props, which are class instances, because their internals should be opaque. Proposed changes: 1. Adding new data type `class_instance`: based on prototype chain of an object we will check if its plain or not. If not, then will be marked as `class_instance`. This should not affect `arrays`, ..., because we do this in the end of an `object` case in `getDataType` function. Important detail: this approach won't work for objects created with `Object.create`, because of the custom prototype. This can also be bypassed by manually deleting a prototype ¯\\\_(ツ)_/¯ I am not sure if there might be a better solution (which will cover all cases) to detect if object is a class instance. Initially I was trying to use `Object.getPrototypeOf(object) === Object.prototype`, but this won't work for cases when we are dealing with `iframe`. 2. Objects with a type `class_instance` will be marked as unserializable and read-only. ## Demo `person` is a class instance, `object` is a plain object https://user-images.githubusercontent.com/28902667/228914791-ebdc8ab0-eb5c-426d-8163-66d56b5e8790.mov
## Summary This adds the ability to create public instances for text nodes in Fabric. The implementation for the public instances lives in React Native (as it does for host components after facebook#26437). The logic here just handles their lazy instantiation when requested via `getPublicInstanceFromInternalInstanceHandle`, which is called by Fabric with information coming from the shadow tree. It's important that the creation of public instances for text nodes is done lazily to avoid regressing memory usage when unused. Instances for text nodes are left intact if the public instance is never accessed. This is necessary to implement access to text nodes in React Native as explained in react-native-community/discussions-and-proposals#607 ## How did you test this change? Added unit tests (also fixed a test that was only testing the logic in a mock :S).
We shouldn't be referencing internal fields like fiber's `flag` directly of DevTools. It's an implementation detail. However, over the years a few of these have snuck in. Because of how DevTools is currently shipped, where it's expected to be backwards compatible with older versions of React, this prevents us from refactoring those fields inside the reconciler. The plan we have to address this is to fix how DevTools is shipped: DevTools will be released in lockstep with each version of React. Until then, though, I need a temporary solution because it's blocking a feature I'm working on. So in meantime, I'm going to have to fork the DevTool's code based on the React version, like we already do with the fiber TypeOfWork enum. As a first step, I've inlined all the references to fiber flags into the specific call sites where they are used. Eventually we'll import these functions from the reconciler so they stay in sync, rather than maintaining duplicate copies of the logic.
…wOpenHandler()` (facebook#26559) ## Summary The electron package was recently upgraded from ^11.1.0 to ^23.1.2 (facebook#26337). However, the WebContents `new-window` event – that is used in the react-devtools project – was deprecated in [v12.0.0](https://releases.electronjs.org/release/v12.0.0) and removed in [v22.2.0](https://releases.electronjs.org/release/v22.2.0). The event was replaced by `webContents.setWindowOpenHandler()`. This PR replaces the `new-window` event with `webContents.setWindowOpenHandler()`. ## How did you test this change? I created a simple electron application with similar functionality: ``` const { app, BrowserWindow, shell } = require('electron') const createWindow = () => { const mainWindow = new BrowserWindow({ width: 800, height: 600 }) mainWindow.webContents.setWindowOpenHandler(({ url }) => { shell.openExternal(url) return { action: 'deny' } }) mainWindow.loadFile('index.html') } app.whenReady().then(() => { createWindow() }) ``` --------- Co-authored-by: root <root@DESKTOP-KCGHLB8.localdomain>
…6563) ## Summary - facebook#26234 is reverted and replaced with a better approach - introduce a new global devtools variable to decouple the global hook's dependency on backend/console.js, and add it to react-devtools-inline and react-devtools-standalone With this PR, I want to introduce a new principle to hook.js: we should always be alert when editing this file and avoid importing from other files. In the past, we try to inline a lot of the implementation because we use `.toString()` to inject this function from the extension (we still have some old comments left). Although it is no longer inlined that way, it has became now more important to keep it clean as it is a de facto global API people are using (9.9K files contains it on Github search as of today). **File size change for extension:** Before: 379K installHook.js After: 21K installHook.js 363K renderer.js
Currently, `waitForThrow` tries to diff the expected value against the thrown value if it doesn't match. However if the expectation is a string, we are not diffing against the thrown message. This commit makes it so if we are matching against message we also diff against message.
facebook#26572) ## Summary This pull request aims to improve the maintainability of the codebase by consolidating types and constants that are shared between the backend and frontend. This consolidation will allow us to maintain backwards compatibility in the frontend in the future. To achieve this, we have moved the shared types and constants to the following blessed files: - react-devtools-shared/src/constants - react-devtools-shared/src/types - react-devtools-shared/src/backend/types - react-devtools-shared/src/backend/NativeStyleEditor/types Please note that the inclusion of NativeStyleEditor in this list is temporary, and we plan to remove it once we have a better plugin system in place. ## How did you test this change? I have tested it by running `yarn flow dom-node`, which reports no errors.
First part of facebook#26571 merging separately to help with git history with a lot of file renames
We have moved away from HostConfig since the name does not fully describe the configs we customize per runtime like FlightClient, FlightServer, Fizz, and Fiber. This commit generalizes $$$hostconfig to $$$config
Many of our Suspense-related tests were written before the `act` API was introduced, and use the lower level `waitFor` helpers instead. So they are less resilient to changes in implementation details than they could be. This converts some of our test suite to use `act` in more places. I found these while working on a PR to expand our fallback throttling mechanism to include all renders that result from a promise resolving, even if there are no more fallbacks in the tree. This isn't all the affected tests, just some of them — I'll be sharding the changes across multiple PRs.
<!-- 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 Addresses facebook#26352. This PR explicitly passes an icon to `chrome.devtools.panels.create()`, so that edge devtools will display the icon when in [Focus Mode](https://learn.microsoft.com/en-us/microsoft-edge/devtools-guide-chromium/experimental-features/focus-mode). <!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? --> ## How did you test this change? Passing test suite (`yarn test` & `yarn test --prod`) ✅ Passing lint (`yarn linc`) ✅ Passing type checks (`yarn flow`) ✅ **Visual Testing** Before Changes | After Changes :-------------------------:|:-------------------------:  |  <!-- 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. -->
…#26604) Fixes facebook#26500 ## Summary - No more using `clipboard-js` from the backend side, now emitting custom `saveToClipboard` event, also adding corresponding listener in `store.js` - Not migrating to `navigator.clipboard` api yet, there were some issues with using it on Chrome, will add more details to facebook#26539 ## How did you test this change? - Tested on Chrome, Firefox, Edge - Tested on standalone electron app: seems like context menu is not expected to work there (cannot right-click on value, the menu is not appearing), other logic (pressing on copy icon) was not changed
…and @NOLINT (facebook#26616) ## Summary We're enabling a new mechanism to synchronize build files from `react` to `react-native`. That new mechanism doesn't post-process files, so we need to add that post-processing somewhere. This PR does that when generating the files in the first place, so the generated files in the `build` directory are ready to be committed in the `react-native` repository directly. This makes use of `signedsource` to avoid direct modifications of these files in the `react-native` repository, as well as `@noformat` and `@nolint` to prevent unactionable CI failures in that repository. ## How did you test this change? Generated build files for `react-native` before and after this change: ``` node ./scripts/rollup/build-all-release-channels.js react-native ``` Checked new contents. Relevant changes: ```diff diff --color -r build-before/react-native/implementations/ReactFabric-dev.fb.js build-after/react-native/implementations/ReactFabric-dev.fb.js 10c10 < * @generated --- > * @generated SignedSource<<03cef14e77b8250b567dfdf3b066085e>> diff --color -r build-before/react-native/implementations/ReactFabric-dev.js build-after/react-native/implementations/ReactFabric-dev.js 11c11 < * @generated --- > * @generated SignedSource<<e39eed38a363846ca9ee9b59a225683c>> diff --color -r build-before/react-native/implementations/ReactFabric-prod.fb.js build-after/react-native/implementations/ReactFabric-prod.fb.js 10c10 < * @generated --- > * @generated SignedSource<<f65efcd6a469d5f6fef1ce647e5ec09a>> diff --color -r build-before/react-native/implementations/ReactFabric-prod.js build-after/react-native/implementations/ReactFabric-prod.js 11c11 < * @generated --- > * @generated SignedSource<<cdd582aa889b1054b2c5faf412622b18>> diff --color -r build-before/react-native/implementations/ReactFabric-profiling.fb.js build-after/react-native/implementations/ReactFabric-profiling.fb.js 10c10 < * @generated --- > * @generated SignedSource<<81e74849b24f104882bd298f062be0fa>> diff --color -r build-before/react-native/implementations/ReactFabric-profiling.js build-after/react-native/implementations/ReactFabric-profiling.js 11c11 < * @generated --- > * @generated SignedSource<<c050b7fa1453dc21ac1c5b98146210a8>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-dev.fb.js build-after/react-native/implementations/ReactNativeRenderer-dev.fb.js 10c10 < * @generated --- > * @generated SignedSource<<9c03464b489b41c06a065aeba8619263>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-dev.js build-after/react-native/implementations/ReactNativeRenderer-dev.js 11c11 < * @generated --- > * @generated SignedSource<<18b34c037544949dcf9b28f945921ba8>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-prod.fb.js build-after/react-native/implementations/ReactNativeRenderer-prod.fb.js 10c10 < * @generated --- > * @generated SignedSource<<592e9654c584d1da523378b119bd8bd7>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-prod.js build-after/react-native/implementations/ReactNativeRenderer-prod.js 11c11 < * @generated --- > * @generated SignedSource<<91c894db99e2d76f8a32708ad6ad1bde>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-profiling.fb.js build-after/react-native/implementations/ReactNativeRenderer-profiling.fb.js 10c10 < * @generated --- > * @generated SignedSource<<5ce378a9216ea747d91b208b9fd1ebd5>> diff --color -r build-before/react-native/implementations/ReactNativeRenderer-profiling.js build-after/react-native/implementations/ReactNativeRenderer-profiling.js 11c11 < * @generated --- > * @generated SignedSource<<1c7564f446ee83142976035b2884dcfd>> diff --color -r build-before/react-native/shims/ReactFabric.js build-after/react-native/shims/ReactFabric.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<cece19ddbec9f287c995721f49c68977>> diff --color -r build-before/react-native/shims/ReactFeatureFlags.js build-after/react-native/shims/ReactFeatureFlags.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<2881c8e89ef0f73f4cf6612cb518b197>> diff --color -r build-before/react-native/shims/ReactNative.js build-after/react-native/shims/ReactNative.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<0debd6e5a17dc037cb4661315a886de6>> diff --color -r build-before/react-native/shims/ReactNativeTypes.js build-after/react-native/shims/ReactNativeTypes.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<652b117c94307244bcf5e4af18928903>> diff --color -r build-before/react-native/shims/ReactNativeViewConfigRegistry.js build-after/react-native/shims/ReactNativeViewConfigRegistry.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<ce82e8957367bee7d11379ab88e3f7c5>> diff --color -r build-before/react-native/shims/createReactNativeComponentClass.js build-after/react-native/shims/createReactNativeComponentClass.js 7c7 < * @Format --- > * @noformat 8a9,10 > * @NOLINT > * @generated SignedSource<<ede54ac2fa1b9a09e234cdf098048989>> ```
…very name (facebook#26624) We currently don't just "require" a module by its module id/path. We encode the pair of module id/path AND its export name. That's because with module splitting, a single original module can end up in two or more separate modules by name. Therefore the manifest files need to encode how to require the whole module as well as how to require each export name. In practice, we don't currently use this because we end up forcing Webpack to deopt and keep it together as a single module, and we don't even have the code in the Webpack plugin to write separate values for each export name. The problem is with CJS we don't statically know what all the export names will be. Since these cases will never be module split, we don't really need to know. This changes the Flight requires to first look for the specific name we're loading and then if that name doesn't exist in the manifest we fallback to looking for the `"*"` name containing the entire module and look for the name in there at runtime. We could probably optimize this a bit if we assume that CJS modules on the server never get built with a name. That way we don't have to do the failed lookup. Additionally, since we've recently merged filepath + name into a single string instead of two values, we now have to split those back out by parsing the string. This is especially unfortunate for server references since those should really not reveal what they are but be a hash or something. The solution might just be to split them back out into two separate fields again. cc @shuding
…26632) This lets the client bundle encode Server References without them first being passed from an RSC payload. Like if you just import `"use server"` from the client. A bundler could already emit these proxies to be called on the client but the subtle difference is that those proxies couldn't be passed back into the server by reference. They have to be registered with React. We don't currently implement importing `"use server"` from client components in the reference implementation. It'd need to expand the Webpack plugin with a loader that rewrites files with the `"use server"` in the client bundle. ``` "use server"; export async function action() { ... } ``` -> ``` import {createServerReference} from "react-server-dom-webpack/client"; import {callServer} from "some-router/call-server"; export const action = createServerReference('1234#action', callServer); ``` The technique I use here is that the compiled output has to call `createServerReference(id, callServer)` with the `$$id` and proxy implementation. We then return a proxy function that is registered with a WeakMap to the particular instance of the Flight Client. This might be hard to implement because it requires emitting module imports to a specific stateful runtime module in the compiler. A benefit is that this ensures that this particular reference is locked to a specific client if there are multiple - e.g. talking to different servers. It's fairly arbitrary whether we use a WeakMap technique (like we do on the client) vs an `$$id` (like we do on the server). Not sure what's best overall. The WeakMap is nice because it doesn't leak implementation details that might be abused to consumers. We should probably pick one and unify.
Full list of changes (not everything included in changelog): * refactor[devtools]: copy to clipboard only on frontend side ([hoxyq](https://github.com/hoxyq) in [facebook#26604](facebook#26604)) * Provide icon to edge devtools. ([harrygz889](https://github.com/harrygz889) in [facebook#26543](facebook#26543)) * [BE] move shared types & constants to consolidated locations ([mondaychen](https://github.com/mondaychen) in [facebook#26572](facebook#26572)) * remove backend dependency from the global hook ([mondaychen](https://github.com/mondaychen) in [facebook#26563](facebook#26563)) * Replace deprecated `new-window` with `webContents.setWindowOpenHandler()` ([Willie-Boy](https://github.com/Willie-Boy) in [facebook#26559](facebook#26559)) * DevTools: Inline references to fiber flags ([acdlite](https://github.com/acdlite) in [facebook#26542](facebook#26542)) * refactor[devtools]: forbid editing class instances in props ([hoxyq](https://github.com/hoxyq) in [facebook#26522](facebook#26522)) * Move update scheduling to microtask ([acdlite](https://github.com/acdlite) in [facebook#26512](facebook#26512)) * Remove unnecessary CIRCLE_CI_API_TOKEN checks ([mondaychen](https://github.com/mondaychen) in [facebook#26499](facebook#26499)) * browser extension: improve script injection logic ([mondaychen](https://github.com/mondaychen) in [facebook#26492](facebook#26492)) * [flow] make Flow suppressions explicit on the error ([kassens](https://github.com/kassens) in [facebook#26487](facebook#26487))
facebook-github-bot
added
CLA Signed
React Core Team
kassens
requested review from
poteto and
rickhanlonii
and removed request for
acdlite
Collaborator
|
If you land it, make sure not to squash because there are renames in there that will get messed up during rebases and blame later. |
Member
Author
|
Superseded by #26639. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 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.
Reverts to 8310854 which was the last successful deployment.
Then re-applies a number of commits that don't impact the fb build flavor and apply cleanly.
The following commits still need to be re-applied with potential fixes:
destroyfield to shared instance objecteventTimesFiber field