Commit Graph

51 Commits

Author SHA1 Message Date
Krzysztof Magiera d16531e8a4 Remove Content-Length header from proxy inspector response (#42590)
Summary:
This change removes Content-Length header from proxy inspector response.

The presence of this header was resulting in the response being cropped under some circumstances because of erroneously calculated length.

The `Content-Length` header value represents the number of bytes in the response. In the code, `string.length` was used to calculate that value, but in JavaScript it gives the number of characters in a string instead of its size in bytes. Specifically, if there are some UTF characters in the string that occupy more than byte, there would be a mismatch in this size. This mismatch resulted in the response being cropped.

The easiest way to reproduce this problem is to set the simulator name to contain a two-byte UTF character.

This change works according to the HTTP spec, which states that when Content-Length is not present, the end of the response stream indicates the end of the response. Since in the code `response.end(data)` is use, it terminates the stream and hence there is no need to provide the length in the header.

## Changelog:

[GENERAL] [FIXED] - fix issue with debugger not working when device name contain two-byte UTF characters

Pull Request resolved: https://github.com/facebook/react-native/pull/42590

Test Plan:
1. Change your iOS simulator name to contain some two-byte UTF character (for example this one: "–")
2. Run metro and connect your app with it
3. Go to http://localhost:8081/json/list in your browser – see the response being marked invalid as it is cropped
4. Apply the change and see that the resulting JSON in the response is now correct
5. Open debugger workflow to confirm it sees the connected device

Reviewed By: robhogan

Differential Revision: D52958725

Pulled By: motiz88

fbshipit-source-id: 92c32893cbbf8552237585d824e4a44737fa3968
2024-01-22 09:26:44 -08:00
Moti Zilberman ca2dde5906 Support launching experimental debugger for modern CDP targets (#42302)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42302

Changelog: [Internal][Added] Support launching experimental debugger frontend for CDP targets marked as "modern"

See the definition of "modern" targets in D50967795.

Reviewed By: hoxyq

Differential Revision: D52786332

fbshipit-source-id: 13718e9ddf3ec050049ef7ec9a77f6cf1a7f82ee
2024-01-18 07:52:51 -08:00
Moti Zilberman 716c728c7a Distinguish between modern/legacy targets, conditionally disable hacks (#42303)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42303

Changelog: [Internal]

Adds a coarse-grained mechanism to `inspector-proxy` for distinguishing between legacy and modern debug targets. The guiding principles are:

1. `inspector-proxy` does not interfere in the CDP message stream between the debugger frontend and a modern target, or in the lifecycle of a target.
2. Legacy runtimes (current React Native, React Native Desktop, etc) that rely on `inspector-proxy`'s existing invasive semantics must continue to work seamlessly for now. We'll decide on the right time to deprecate/remove this legacy code in the future.

NOTE: This is an experimental addition to the proxy protocol that may be replaced at any time.

Reviewed By: hoxyq

Differential Revision: D50967795

fbshipit-source-id: bb9c39a8fe755ef3661e2c61507dd324d8dc8894
2024-01-18 07:52:51 -08:00
Moti Zilberman 8ac08c8d18 Deduplicate pages by ID within each device (#42282)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42282

Changelog: [Internal] `inspector-proxy` now assumes each app will report pages with locally unique IDs.

In order to simplify some upcoming logic changes in `inspector-proxy`, in this diff we begin to enforce the assumption that each app ( = platform-specific implementation of `InspectorPackagerConnection`) assigns a locally unique ID to each inspector page. The inspector proxy will silently drop page descriptors that have conflicting IDs, and log a message to `debug()`.

NOTE: As an implementation detail, integrators may use `DEBUG=Metro:InspectorProxy` to see debug messages from `inspector-proxy`.

Reviewed By: huntie

Differential Revision: D50969752

fbshipit-source-id: a4e6faa91d97594fc5343ce4bee66233523cd175
2024-01-17 22:53:42 -08:00
Moti Zilberman 7a69fe424d Migrate to private member syntax (#42299)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/42299

Changelog: [Internal]

TSIA

Reviewed By: hoxyq

Differential Revision: D52798108

fbshipit-source-id: 9ba980b9fb439b1aeee3eb1b08535b95556d72bb
2024-01-17 08:03:34 -08:00
Ian Park 46d304a101 Add 127.0.0.1 to allowlist for source map fetch
Summary:
Adds ipv4 localhost representation (127.0.0.1) to allowlisted hostnames for the inspector-proxy, to allow for proxied network requests fetching source maps.

Changelog:
[General][Fixed] Allow source map fetching in the inspector proxy from 127.0.0.1

Reviewed By: motiz88

Differential Revision: D52112849

fbshipit-source-id: e6d45fa2fdfd570b7ab4f36121ba615c9b06d442
2023-12-13 04:38:17 -08:00
Oskar Kwaśniewski 7d1a98c43b Lint source files, sort imports, remove unused imports (#41829)
Summary:
This PR lints source files using eslint. I've executed `yarn lint --fix` and also manually fixed some of eslint issues.

Before:

![CleanShot 2023-12-07 at 12 07 10@2x](https://github.com/facebook/react-native/assets/52801365/2b00cf23-e5a0-46b8-802f-adcb67224111)

After:

![CleanShot 2023-12-07 at 12 06 24@2x](https://github.com/facebook/react-native/assets/52801365/bb05b2c0-2b27-4f99-b7b4-cb47a51a3885)

## Changelog:

[GENERAL] [FIXED] - Lint source files, sort imports, remove unused ones

Pull Request resolved: https://github.com/facebook/react-native/pull/41829

Test Plan: CI Green

Reviewed By: christophpurrer

Differential Revision: D51979074

Pulled By: dmytrorykun

fbshipit-source-id: e11b90721e33f5e9949a0833e5f39fe7ba3d1067
2023-12-11 08:54:29 -08:00
Cedric van Putten 8ef807bfb2 feature(dev-middleware): add enableNetworkInspector experiment (#41787)
Summary:
This enables the network panel/inspector by passing the `unstable_enableNetworkPanel=true` to the React Native JS Inspector. (See https://github.com/facebookexperimental/rn-chrome-devtools-frontend/pull/2)

By setting this inside the `experiments`, we can enable/disable network related CDP handlers within the proxy.

## Changelog:

[GENERAL] [ADDED] - Add `enableNetworkInspector` experiment to enable Network panel and CDP handlers in inspector proxy

Pull Request resolved: https://github.com/facebook/react-native/pull/41787

Test Plan: TBD, will provide a repository using an Expo canary / RN 0.73.0-rc release.

Reviewed By: NickGerleman

Differential Revision: D51811892

Pulled By: huntie

fbshipit-source-id: 541d96b6f0735104a4050a24a152e1158871ed1d
2023-12-07 08:30:03 -08:00
Gijs Weterings 73411f4e08 Back out "Restore InspectorProxy tests - fix Windows by Babel-ignoring node_modules consistently"
Summary: Changelog: Internal

Reviewed By: gsathya

Differential Revision: D51447333

fbshipit-source-id: 56eb839671ca62269d59d48fd3c83df4f42a5111
2023-11-18 02:04:44 -08:00
Rob Hogan 59a2282b29 Restore InspectorProxy tests - fix Windows by Babel-ignoring node_modules consistently (#41526)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41526

CI failures in Windows JS tests recently (https://github.com/facebook/react-native/pull/41463) were caused by the triggering of Babel registration during tests, due to an import of `packages/dev-middleware` (index), breaking subsequent transformation of other tests.

## Root cause
Example of a problematic import:
https://github.com/facebook/react-native/blob/a5d8ea4579c630af1e4e0fe1d99ad9dc0915df86/packages/dev-middleware/src/__tests__/ServerUtils.js#L15

..which triggers a Babel registration:
https://github.com/facebook/react-native/blob/a5d8ea4579c630af1e4e0fe1d99ad9dc0915df86/packages/dev-middleware/src/index.js#L16-L18

That registration behaves differently on Windows due to the `ignore: [/\/node_modules\/\]`, which doesn't match against Windows path separators - Babel matches against system separators.

In particular, this changed whether `node_modules/flow-parser` was transformed when loading the RN Babel transformer. Transforming this file causes a `console.warn` from Babel due to its size:
> [BABEL] Note: The code generator has deoptimised the styling of /Users/robhogan/workspace/react-native/node_modules/flow-parser/flow_parser.js as it exceeds the max of 500KB.

This throws due to our setup:
https://github.com/facebook/react-native/blob/a5d8ea4579c630af1e4e0fe1d99ad9dc0915df86/packages/react-native/jest/local-setup.js#L27

This all manifests as the first test following a Babel registration (within the same Jest worker) that requires the RN Babel transformer throwing during script transformation.

## This change
This is the minimally disruptive change that makes Babel registration behaviour consistent between Windows and other platforms. The more durable solution here would be *not* to rely on any Babel registration for Jest, which has its own `ScriptTransformer` mechanism for running code from source. Given the fragile way our internal+OSS Babel set up hangs together that's a higher-risk change, so I'll follow up separately.

Changelog: [Internal]

Reviewed By: huntie

Differential Revision: D51424802

fbshipit-source-id: 8b733c0c159ee84690aef04abced682d126c6d27
2023-11-17 08:53:14 -08:00
Moti Zilberman 560e0f0005 Migrate to @rnx-kit/chromium-edge-launcher for Windows fix (#41367)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41367

Andrew Coates graciously published the Windows Edge launcher fix from https://github.com/cezaraugusto/chromium-edge-launcher/pull/1 as a new package (https://github.com/microsoft/rnx-kit/pull/2796), so let's pull that into `dev-middleware`.

Changelog: [Internal] - Fix experimental debugger launch flow with Edge on Windows

Reviewed By: robhogan

Differential Revision: D51086297

fbshipit-source-id: 3a8db351f71eb31a9609c987cdb4dc66f24f9403
2023-11-15 00:18:10 -08:00
Gijs Weterings 453e5c0d09 Temporarily disable InspectorProxy.* tests to unblock CI (#41463)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41463

S378983 Circle CI tests have been red for 5 days. There's a test setup issue somewhere in this test suite, until motiz88 can determine where exactly, let's disable them

T169943794 filed to follow up

Changelog: Internal

Reviewed By: cipolleschi

Differential Revision: D51271630

fbshipit-source-id: 7dbc61bb4c8df0d5360ba239a1f00c4270a691f3
2023-11-13 13:24:47 -08:00
Moti Zilberman 4c0d20dfbf Fix platform sensitivity in inspector-proxy tests (#41439)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41439

TSIA - should fix the [Windows test failure](https://app.circleci.com/pipelines/github/facebook/react-native/36174/workflows/055cabb6-cb41-4741-812a-59b06fc7c9b5/jobs/1200335/parallel-runs/0/steps/0-115?fbclid=IwAR38F8tE-831Rpe5J_KeFOmA5DARLWWYIAiLIgkXOLdEfMmSeaVOIGgPq5U) we saw on CircleCI.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D51254843

fbshipit-source-id: d45682c460d54b3d76d3a02416b75f0c11313fd7
2023-11-13 05:12:00 -08:00
Gabriel Donadel 1a61afddf7 Expose unstable_InspectorProxy and unstable_Device from dev-middleware (#41370)
Summary:
Recently, both `metro-inspector-proxy`(https://github.com/facebook/react-native/pull/39045) and `react-native-community/cli-plugin-metro`(https://github.com/facebook/react-native/pull/38795) were moved to this repo and in the process of moving these packages, the `exports` field inside package.json was added, only exporting the `index.js` file.

The problem is that Expo CLI (and possibly other community packages) rely on functions and classes that are not exported in the `index.js` file, e.g. Importing the InspectorProxy class from `react-native/dev-middleware/dist/inspector-proxy/InspectorProxy`. Normally this wouldn't be a problem and we would just import from `dist/` but due to the `exports` field, attempting to import from any other file not specified on this field will result in a `ERR_PACKAGE_PATH_NOT_EXPORTED` error.

As a short-term fix, we should create `unstable_`-prefixed exports of individual features Expo currently depends on.

## Changelog:

[INTERNAL] [CHANGED] - Expose unstable_InspectorProxy and unstable_Device from `react-native/dev-middleware`

Pull Request resolved: https://github.com/facebook/react-native/pull/41370

Test Plan: N / A

Reviewed By: robhogan

Differential Revision: D51163134

Pulled By: blakef

fbshipit-source-id: e67adaedc4fc64131e4c9dd8383c9877b8202283
2023-11-10 04:45:51 -08:00
Moti Zilberman 1bcd28636f Handle source file / source map fetch errors correctly, add tests (#41342)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41342

While rewriting `Debugger.getScriptSource` messages to fetch code and source map over HTTP, we weren't checking the status code of the fetch calls. This diff fixes that and adds corresponding tests (as well as for the filesystem error case).

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D51013054

fbshipit-source-id: 58e7bb9fcd6a3cf92329b43fb8a139093c80d305
2023-11-08 07:19:40 -08:00
Moti Zilberman 4eb3e300a4 Add tests for reload message injection (#41340)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41340

Tests for D17100911 and D14503522.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D51013053

fbshipit-source-id: 65aba42e21fad891e458647c8421cd316518ec3c
2023-11-08 07:19:40 -08:00
Moti Zilberman 246657214d Add tests for Debugger.getScriptSource rewriting (#41338)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41338

Tests for D21401320 and D39629274.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D51013052

fbshipit-source-id: af25935d1d7fff90112720888b3728ce195ac5d1
2023-11-08 07:19:40 -08:00
Moti Zilberman 3afadef6d6 Add tests for source map fetching and URL rewriting hacks (#41339)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41339

Tests for D42973408, D14800976, and D15116579.

Changelog: [Internal]

Reviewed By: blakef

Differential Revision: D51013055

fbshipit-source-id: cff8a9d1dd22b642117594da16e6b74cf679aa34
2023-11-08 07:19:40 -08:00
Moti Zilberman c6eccda2c7 Add tests for HTTPS-specific behaviour (#41341)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41341

* Extends `dev-middleware`'s test utilities to enable testing against an HTTPS server with a self-signed cert.
* Runs the CDP transport integration tests (D51002261) using both HTTP and HTTPS.
* Adds a test to explicitly cover the `ws=...` / `wss=...` variation in `devtoolsFrontendUrl` first introduced in D49158227.

Changelog: [Internal]

Reviewed By: blakef

Differential Revision: D51006835

fbshipit-source-id: df3db8cd865898248cd0d8f307f75949a7f313fd
2023-11-08 07:19:40 -08:00
Moti Zilberman c85b2da1e7 Add minimal test for "Improved Chrome Reloads" synthetic page (#41335)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41335

`inspector-proxy` has special behaviour to allow a debugger connection to persist across app reloads.

In the React Native runtime, a reload is modelled as the creation of an entirely new "page" with its own ID. To insulate the debugger from this detail, the proxy advertises a separate, synthetic page on each device, with ID `-1`, that always maps to the latest React Native page reported by that device.

Here we test the message forwarding part of this functionality. The proxy also injects CDP messages (in both directions) as part of simulating a reload, but that will be tested in a separate diff.

Changelog: [Internal]

Reviewed By: blakef

Differential Revision: D51002262

fbshipit-source-id: 296135177321a511ebbe7d9696e4e7a61275aa32
2023-11-08 07:19:40 -08:00
Moti Zilberman 38a878194b Add CDP transport tests (#41343)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41343

Changelog: [Internal]

Adds basic tests for two-way communication between a debugger (frontend) and a target (backend) using CDP over `inspector-proxy`.

Reviewed By: blakef

Differential Revision: D51002261

fbshipit-source-id: 44e571f89437c26e76ef6e6192b2bf6244665cf0
2023-11-08 07:19:40 -08:00
Moti Zilberman 1602d42cec Clean up polling interval on device disconnection (#41331)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41331

`inspector-proxy`'s `Device` class currently leaks a `setInterval` handle. This is mostly harmless in current usage. In the test suite added up the stack, it shows up as a leak that prevents Jest from exiting cleanly, so let's clean it up properly.

Changelog: [Internal]

Reviewed By: blakef

Differential Revision: D51002263

fbshipit-source-id: ca36797ce1196aa049ceb3a8e96ee53d34893fdc
2023-11-08 07:19:40 -08:00
Moti Zilberman 0ea72807a6 Add tests for inspector-proxy's HTTP API (#41314)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41314

Changelog: [Internal]

Adds the beginning of a test suite for `inspector-proxy`. For maintainability, we only test functionality exposed from the `dev-middleware` boundary rather than instantiating `InspectorProxy` directly.

In this diff, the test coverage is far from complete, but this is a first stab at covering some basics. `InspectorProxyHttpApi-test` exercises the HTTP GET endpoints (`/json/list` and `/json/version`) as well as some device registration logic through the `/inspector/device` WebSocket. Some reusable helpers for server setup and device mocking are included in separate files.

As an overall strategy, I'm planning to add multiple test files that share helpers between them, not build out one massive test file with all the helpers inline. There will likely be some verbose tests when we start covering debugger-to-device communication, and I want to keep them as readable as possible.

Reviewed By: blakef

Differential Revision: D50980467

fbshipit-source-id: 962dae5a380451d6dac57eac23c4436550a39cf8
2023-11-08 07:19:40 -08:00
Moti Zilberman d6e0bc714a Enable lint/sort-imports everywhere (#41334)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41334

TSIA.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D51025812

fbshipit-source-id: e10d437be775a6b80946483aa96460f34927f870
2023-11-06 12:59:38 -08:00
Moti Zilberman 1d6b0f1420 Use exact, read-only types for protocol data structures (#41315)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41315

TSIA

Changelog: [Internal]

Reviewed By: hoxyq

Differential Revision: D50980466

fbshipit-source-id: 1f289b868f5c735396fe07899c558e417a4ee2ff
2023-11-06 11:01:02 -08:00
Moti Zilberman 7009634d38 Enable launching debugger by device ID (#41154)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41154

Pull Request resolved: https://github.com/facebook/react-native/pull/41080

Building on byCedric's approach in https://github.com/facebook/metro/pull/991, adds support for passing a `device=...` argument to `/open-debugger` for more precise targeting.

Changelog: [Internal]

 ---

## Note on what "device" means in this context

In `dev-middleware` / `inspector-proxy`, "device" is something of a misnomer. It refers to a *logical device* containing one or more *pages*. In React Native, each app process forms its own logical device in which individual VMs register themselves as pages. An instance of `inspector-proxy` connects one or more *debuggers* (frontends) to one or more logical devices (one frontend to one page on one device).

The intent of the logical device ID is to help with target discovery and especially *re*discovery - to reduce the number of times users need to explicitly close and restart the debugger frontend (e.g. after an app crash).

If provided, the logical device ID:
1. SHOULD be stable for the current combination of physical device (or emulator instance) and app.
2. SHOULD be stable across installs/launches of the same app on the same device (or emulator instance), though it MAY be user-resettable (so as to not require any special privacy permissions).
3. MUST be unique across different apps on the same physical device (or emulator).
4. MUST be unique across physical devices (or emulators).
5. MUST be unique for each concurrent *instance* of the same app on the same physical device (or emulator).

NOTE: The uniqueness requirements are stronger (MUST) than the stability requirements (SHOULD). In particular, on platforms that allow multiple instances of the same app to run concurrently, requirements 1 and/or 2 MAY be violated in order to meet requirement 5. This will be relevant, for example, on desktop platforms.

In an upcoming diff, we will pass device IDs meeting these criteria from both iOS and Android.

Reviewed By: huntie, blakef

Differential Revision: D49954920

fbshipit-source-id: 45f2b50765dece34cbb93fa32abcdf3b0522391c
2023-10-23 05:50:36 -07:00
Gijs Weterings 67c1a806e6 Flow strictify xplat/js/react-native-github where possible (#41051)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41051

Strictifies flow to flow strict-local in files where doing that doesn't cause new flow errors.

Changelog: Internal

Reviewed By: yungsters

Differential Revision: D50369011

fbshipit-source-id: b4a5a26b839b7327a3178e6f5b35246dea365a38
2023-10-18 05:36:33 -07:00
116-7 ab81c16b65 Refactors InspectorProxy to compare pathname portion of url passed in… (#41005)
Summary:
… request to local pathname comparator variables to fix issue with other rightward elements of url such as query or fragment entering the comparison and causing 404 errors for key debugging routes.

A change in Chromium appended the query "?for_tabs" to the /json/list request made by Chrome DevTools to find remote debugger targets.

The current comparison in InspectorProxy.js compares the entire node IncomingMessage url field to the local pathname constants. The issue arises as url can also contain the query and fragment portions so the original comparison of "/json/list" === "/json/list" which resolved as true would become "/json/list?for_tabs" === "/json/list" and evaluate to false ultimately resulting in a 404 for the request.

In summary, all these changes/issues caused remote debugging of Hermes code in React Native apps to become unavailable, greatly impacting developer experience.

## Changelog:

[GENERAL] [FIXED] JS Debugging: Fix inspector-proxy to allow for DevTools requests with query strings

Pull Request resolved: https://github.com/facebook/react-native/pull/41005

Reviewed By: NickGerleman

Differential Revision: D50342265

Pulled By: robhogan

fbshipit-source-id: a65f2908f0bea9fc15e1e3e4e6d31a3b9598e81f
2023-10-17 10:08:05 -07:00
Alex Hunt 9e068ac163 Add --experimental-debugger-frontend flag, restore 0.72 flow as base (#40766)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/40766

This changeset allows users to opt into the new debugger frontend experience by passing `--experimental-debugger` to `react-native start`. **We are defaulting this option to `true`** for now, but will continue to evaluate this feature before 0.73 ships. It restores Flipper (via `flipper://`) as the default handling for `/open-debugger` (matching 0.72 behaviour) when this flag is not enabled.

Detailed changes:

- Replaces `enableCustomDebuggerFrontend` experiment in `dev-middleware` with `enableNewDebugger`. The latter now hard-swaps between the Flipper and new launch flows.
    - Removes now-unused switching of `devtoolsFrontendUrl`.
- Implements `deprecated_openFlipperMiddleware` (matching previous RN CLI implementation).
- Disables "`j` to debug" key handler by default.
- Marks "`j` to debug" and `/open-debugger` console logs as experimental.

Changelog:
[Changed][General] Gate new debugger frontend behind `--experimental-debugger` flag, restore Flipper as base launch flow

Reviewed By: motiz88

Differential Revision: D50084590

fbshipit-source-id: 5234634f20110cb7933b1787bd2c86f645411fff
2023-10-10 09:37:41 -07:00
Alex Hunt 4dcd5f1065 Type serve-static and document /debugger-frontend (#40765)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/40765

Types based on https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/serve-static/index.d.ts.

Changelog: [Internal]

Reviewed By: motiz88

Differential Revision: D50084591

fbshipit-source-id: 92fd833d90dfc5acbd3be0476f1da34e5742a732
2023-10-10 09:37:41 -07:00
Moti Zilberman d0f750e15d Disable max WebSocket message size validation in CDP proxy (#39833)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39833

Reland of D49642047 which got reverted because of a CI infra error.

 ---

It's currently possible for RN to crash the dev server by sending down an exceptionally large CDP response/event. Instead of making assumptions on the protocol spoken over the proxy, let's assume clients on either side of the proxy can be trusted to be well behaved (and to degrade gracefully when a large message is encountered).

Changelog: [General][Fixed] JS debugging: prevent dev server crash when a large CDP payload is returned from the device

Reviewed By: huntie

Differential Revision: D49955025

fbshipit-source-id: aa5b8b55c885e26dd5b8170660603173cfe54de0
2023-10-06 06:44:43 -07:00
Hakeem King 7d88ac4ced Revert D49642047: Disable max WebSocket message size validation in CDP proxy
Differential Revision:
D49642047

Original commit changeset: 07b134c9fa6a

Original Phabricator Diff: D49642047

fbshipit-source-id: f72f7c72cf1a7941199c3d49f1a2edccb0410ac5
2023-10-04 17:48:12 -07:00
Moti Zilberman 2000acc6c6 Disable max WebSocket message size validation in CDP proxy (#39809)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39809

It's currently possible for RN to crash the dev server by sending down an exceptionally large CDP response/event. Instead of making assumptions on the protocol spoken over the proxy, let's assume clients on either side of the proxy can be trusted to be well behaved (and to degrade gracefully when a large message is encountered).

Changelog: [General][Fixed] JS debugging: prevent dev server crash when a large CDP payload is returned from the device

Reviewed By: huntie

Differential Revision: D49642047

fbshipit-source-id: 07b134c9fa6aba7ce2208f71981d6d862281395f
2023-10-04 11:57:43 -07:00
Alex Hunt 850e550422 Add serverBaseUrl option, set client-accessible URL value externally (#39456)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39456

**Fixes new debugger launch flow on Android:**

D49158227 aimed to improve proxy-safe behaviour for remote dev servers by auto-detecting the appropriate server URL for clients using the `Host` header (etc) from the HTTP request. However, this approach broke the local case for Android emulators and externally connected devices since they would originate from a device-relative server hostname — e.g. `10.0.2.2` for the stock Android emulator.

https://pxl.cl/3mVmR

This commit reverts to an explicit approach where callers specify the base URL to the dev server that should be addressible from the development machine — now as a single `serverBaseUrl` option.

**Changes**

- Adds new `serverBaseUrl` option to `createDevMiddleware`, designed to be the base URL value for constructing dev server URLs returned in endpoints such as `/json/list`.
    - This changes little for the `localhost` case (now enabling `https://` URLs), but enables remote dev server setups to specify this cleanly.
- Updates call site in `community-cli-plugin`.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D49276125

fbshipit-source-id: 2b6a8507073649832993971aa9d0870f54c9bd44
2023-09-15 13:11:04 -07:00
Moti Zilberman 6038598302 Experimentally support GET requests in /open-debugger (#39418)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39418

Changelog: [Internal]

Adds the `enableOpenDebuggerRedirect` experiment flag. The flag enables the handling of GET requests in the `/open-debugger` endpoint, in addition to POST requests. GET requests respond by redirecting to the debugger frontend, instead of opening it using the `BrowserLauncher` interface.

This can be useful when integrating `dev-middleware` in highly customised environments (e.g. VS Code remoting) where things like automatic port forwarding interact poorly with the `BrowserLauncher` abstraction.

WARNING: As this is gated by an experiment flag, the functionality may change or go away unannounced. In separate work, we will look into a stable solution for this use case.

Reviewed By: huntie

Differential Revision: D49144733

fbshipit-source-id: 5af6c8b2ddaa7b6e7d14c792e49fe3d0849c7a25
2023-09-14 06:08:57 -07:00
Moti Zilberman b8e42ea7b4 Build client-accessible frontend URLs in /open-debugger (#39423)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39423

D49158227 made the CDP WebSocket URLs returned from `/json` proxy-safe by using the Host header (etc) from the HTTP request. This did *not* fully fix the related `/open-debugger` endpoint, because the original headers were being lost in the internal `fetch('/json')` call.

Here, we eliminate that `fetch` call, and instead give the `/open-debugger` handler direct access to the method on `InspectorProxy` that backs the `/json` endpoint. This allows us to pass in an appropriate base URL derived from the headers seen by `/open-debugger`.

Changelog: [Internal]

Reviewed By: huntie

Differential Revision: D49229545

fbshipit-source-id: 9036ab295721e0d1fd3cdb608d0a7cc07b8f2eeb
2023-09-14 06:08:57 -07:00
Moti Zilberman 34a7526e79 Remove host/port params, construct WebSocket URL from /json request (#39394)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39394

* Constructs DevTools frontend WebSocket URLs with a client-accessible host, port and scheme derived from the initiating HTTP request (`/json` or `/open-debugger`), instead of from the static `host` and `port` parameters provided to `createDevMiddleware`. This results in more proxy-safe behaviour.
* Removes the now-unused `host` and `port` parameters from `createDevMiddleware`.

Changelog: [Internal]

Reviewed By: huntie

Differential Revision: D49158227

fbshipit-source-id: ec61d98458e5d5afba4eb937b84ff65071495cc9
2023-09-12 04:57:58 -07:00
Moti Zilberman 3ec22c1e69 Add option to enable experimental debugger frontend (#39227)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39227

Changelog: [Internal]

1. Adds an `unstable_experiments` option to `createDevMiddleware` in `react-native/dev-middleware`.
2. Adds `enableCustomDebuggerFrontend` (default `false` for now) as an experiment flag controlling whether the new debugger frontend (D48680624, D48682302) is in use. We plan to enable this by default in RN 0.73 after additional testing.

If enabled, the new debugger frontend will only be used for the `/open-debugger` flow

Reviewed By: huntie

Differential Revision: D48602725

fbshipit-source-id: 598865b559478df1f19420daf3633ee6c233362a
2023-09-04 12:21:47 -07:00
Alex Hunt f688a2d4be Update /open-debugger to try first target when unspecified (#39255)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39255

Changelog: [Internal]

Reviewed By: motiz88

Differential Revision: D48873336

fbshipit-source-id: 8d3aac383de1cefa303a89fbfee9a23ce906a979
2023-09-04 06:36:42 -07:00
Alex Hunt a5fd0f17e9 Revert to built-in devToolsFrontendUrl in target list (#39242)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39242

Reverts external behaviour of https://github.com/facebook/react-native/pull/39122.

- `/json/list` once again returns the built-in `devtools://devtools/bundled/js_app.html` URL — effectively freezing the current experience in Flipper.
- `/open-debugger` continues to use the *new* frontend via `getDevToolsFrontendUrl`.
    - *Eventually*, we'll want to align this as the returned `devtoolsFrontendUrl` value once Flipper is out of the picture.

Changelog: [Internal]

Reviewed By: motiz88

Differential Revision: D48868767

fbshipit-source-id: 088ecebffa6ce45cb9bed44e3b02df4b7b471068
2023-08-31 09:36:00 -07:00
Alex Hunt dab7738a88 Expose unstable_browserLauncher option
Summary:
Expose a `unstable_browserLauncher` option to `createDevMiddleware()`. This allows integrators to provide a custom implementation for opening URLs in a web browser, initially a `launchDebuggerAppWindow` method.

Customising the browser launcher implementation can be useful in cases where the dev server is running remotely and not on the developer's local machine.

Changelog: [Internal]

Reviewed By: motiz88

Differential Revision: D48647750

fbshipit-source-id: ebf34106ad27899024396b13b22ec4536aad05b9
2023-08-25 10:02:12 -07:00
Alex Hunt 4cb9706331 Add Microsoft Edge support for opening debugger (#39146)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39146

Adds Microsoft Edge fallback for the `/open-debugger` endpoint when no Chrome installation is found.

Changelog: [Internal]

Reviewed By: motiz88

Differential Revision: D48563386

fbshipit-source-id: 74baba7c03a062bd769b2f9ac0cc35bac0b2ae65
2023-08-24 10:57:39 -07:00
Alex Hunt f9e936e280 Update devtoolsFrontendUrl value in target list (#39122)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39122

Updates the `devtoolsFrontendUrl` value returned in the `/json/list` endpoint, to match the fixed DevTools frontend revision we set for `/open-debugger` — which now uses this as the source of truth.

Changelog: [Internal]

Reviewed By: motiz88

Differential Revision: D48561005

fbshipit-source-id: 19409d013366f82782a071a0ec01d1ce8fab8c38
2023-08-23 04:17:12 -07:00
Moti Zilberman 8b62200605 Annotate debugger events with frontend User-Agent if available (#39110)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39110

TSIA.

Changelog: [Internal]

Reviewed By: huntie

Differential Revision: D48560563

fbshipit-source-id: a268173e5e7b3e6e497ab79a8b362caa897afec6
2023-08-22 11:11:50 -07:00
Moti Zilberman cbbf169b06 Annotate debugger events with app ID, device name etc (#39108)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39108

TSIA

Changelog: [Internal]

Reviewed By: huntie

Differential Revision: D48484712

fbshipit-source-id: 3d875fdd098f2ef98b3bccb03f0053d0e5cc3be7
2023-08-22 11:11:50 -07:00
Moti Zilberman d10a8098b2 Log debugger command latency and status (#39107)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39107

Augments D48466760 to report debugger commands (request/response pairs) through `EventReporter`. With the appropriate backend integration, this can help validate the correctness/completeness of the CDP implementation (in the client, proxy and server) and measure the latency of debugger commands - more accurately, the time between the proxy receiving a command from the client and receiving the corresponding response from the server.

Most of the new logic here is in the `DeviceEventReporter` class, which is responsible for associating responses with requests. Requests are kept in a buffer with a 10s TTL which serves as a timeout in case of an unresponsive server.

Changelog: [Internal]

Reviewed By: huntie

Differential Revision: D48480372

fbshipit-source-id: 55360a14bbea05ef8ad1622e0d54f18b47483809
2023-08-22 11:11:50 -07:00
Moti Zilberman f4f18940d7 Create unstable API for event logging (#39091)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39091

Changelog: [Internal]

Adds a simple typed logging hook to `react-native/dev-middleware`. This is intended to allow integrators to receive events from the dev server, apply any relevant sampling/processing, and log them to a backend. (To be clear, the open source version of React Native does not and will not collect any data.)

WARNING: The API will evolve over the coming weeks/months and is *not guaranteed to be stable* - it might even break between patch releases.

Reviewed By: huntie

Differential Revision: D48466760

fbshipit-source-id: ed1e21fb0dac5d6199ff1ee26017a1d33d9b7d92
2023-08-21 10:31:50 -07:00
Alex Hunt c82cf64a22 Move metro-inspector-proxy into dev-middleware (#39045)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/39045

## Context

RFC: Decoupling Flipper from React Native core: https://github.com/react-native-community/discussions-and-proposals/pull/641

## Changes

- Relocates `metro-inspector-proxy` source from the Metro repo into the React Native repo as part of the `react-native/dev-middleware` package.
    - Drops the `runInspectorProxy` entry point.
- Attaches the Inspector Proxy to the `createDevMiddleware()` API as the new integration point for this functionality.
- Documents migrated endpoints + usage of `createDevMiddleware()` in README.

Changelog: [Internal]
Metro changelog: None (`metro-inspector-proxy` is now an internal component of `react-native`, covered in the [release notes for 0.78.1](https://github.com/facebook/metro/releases/tag/v0.78.1))

Reviewed By: motiz88, blakef

Differential Revision: D48066213

fbshipit-source-id: 3fbef5d881f6f451cb5955dcbbc362c53347437e
2023-08-18 01:38:10 -07:00
Alex Hunt a978d343a6 Add auto-generation of TypeScript definitions on build (#38990)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38990

This PR adds auto-generation of Typescript definitions from Flow source code for packages using the shared monorepo build setup (https://github.com/facebook/react-native/pull/38718).

Today, these are the following Node.js packages:

- `packages/community-cli-plugin`
- `packages/dev-middleware` (⬅️ `emitTypeScriptDefs` enabled)

This also improves emitted Flow definitions (`.js.flow`), by using [`flow-api-translator`](https://www.npmjs.com/package/flow-api-translator) to strip implementations.

**All changes**

- Include `flow-api-translator` and configure this to emit type definitions as part of `yarn build`.
    - Add translation from Flow source to TypeScript definitions (`.d.ts`) adjacent to each built file.
    - Improve emitted Flow definitions (`.js.flow`), by using `flow-api-translator` to strip implementations (previously, source files were copied). The Flow and TS defs now mirror each other.
-  Add `emitFlowDefs` and `emitTypeScriptDefs` options to build config to configure the above.
- Integrate TypeScript compiler to perform program validation on emitted `.d.ts` files.
     - This is based on this guide: https://github.com/microsoft/TypeScript-wiki/blob/main/Using-the-Compiler-API.md#a-minimal-compiler.
- Throw an exception on the `rewritePackageExports` step if a package does not define an `"exports"` field.
- Add minimal `flow-typed` definitions for `typescript` 😄.

**Notes on [`flow-api-translator`](https://www.npmjs.com/package/flow-api-translator)**

This project is experimental but is in a more mature state than when we evaluated it earlier in 2023.
- It's now possible to run this tool on our new Node.js packages, since they are exclusively authored using `import`/`export` syntax (a requirement of the tool).
- As a safety net, we run the TypeScript compiler against the generated program, which will fail the build.

Changelog: [Internal]

Reviewed By: robhogan

Differential Revision: D48312463

fbshipit-source-id: 817edb35f911f52fa987946f2d8fc1a319078c9d
2023-08-14 12:12:10 -07:00
Alex Hunt cd8f5d176a Add shared monorepo build setup (#38718)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38718

> NOTE: Replaces https://github.com/facebook/react-native/pull/38240

## Context

RFC: Decoupling Flipper from React Native core: https://github.com/react-native-community/discussions-and-proposals/pull/641

## Changes

To support incoming new React Native packages around debugging (including migrating over [`react-native-community/cli-plugin-metro`](https://github.com/react-native-community/cli/tree/main/packages/cli-plugin-metro)) — which target Node.js and require a build step, this PR adds a minimal shared build setup across the `react-native` monorepo.

The setup is closely inspired/based on the build scripts in Jest, Metro, and React Native CLI — and is a simple set of script wrappers around Babel. These are available as build commands at the root of the repo:

- `yarn build` — Builds all configured packages. Functionally, this:
  - Outputs a `dist/` directory with built files.
  - Rewrites package.json `"exports"` to update every `./src/*` reference to `./dist/*` (source of truth).
- `scripts/build/babel-register.js` — Allows running all Node.js entry points from source, similar to the current setup in [facebook/metro](https://github.com/facebook/metro). (Example entry point file in this PR: `packages/dev-middleware/src/index.js`)

Build configuration (i.e. Babel config) is shared as a set standard across the monorepo, and **packages are opted-in to requiring a build**, configured in `scripts/build.config.js`.

```
const buildConfig /*: BuildConfig */ = {
  // The packages to include for build and their build options
  packages: {
    'dev-middleware': {target: 'node'},
  },
};
```

For now, there is a single `target: 'node'` option — this is necessary as `react-native`, unlike the above other projects, is a repository with packages targeting several runtimes. We may, in future, introduce a build step for other, non-Node, packages — which may be useful for things such as auto-generated TypeScript definitions.

 {F1043312771}

**Differences from the Metro setup**

- References (and compiles out) repo-local `scripts/build/babel-register.js` — removing need for an npm-published dependency.

## Current integration points

- **CircleCI** — `yarn build` is added to the `build_npm_package` and `find_and_publish_bumped_packages` jobs.

**New Node.js package(s) are not load bearing quite yet**: There are not yet any built packages added to the dependencies of `packages/react-native/`, so this will be further tested in a later PR (and is actively being done in an internal commit stack).

### Alternative designs

**Per-package config file**

Replace `scripts/build/config.js` with a package-defined key in in `package.json`, similar to Jest's [`publishConfig`](https://github.com/jestjs/jest/blob/1f019afdcdfc54a6664908bb45f343db4e3d0848/packages/jest-cli/package.json#L87C3-L89C4).

```
"buildConfig": {
  "type": "node"
},
```

This would be the only customisation required, with a single Babel config still standardised. Another option this might receive in future is `enableTypeScriptCodgeen`.

**Rollup**

More sophisticated build tool for Node.js, used by the React codebase (albeit within a custom script setup as well).

**Lerna and Nx**

- Most sophisticated setup enabling caching and optimised cloud runs.
- Probably the most likely thing we'll move towards at a later stage.

Changelog: [Internal]

Reviewed By: NickGerleman

Differential Revision: D47760330

fbshipit-source-id: 38ec94708ce3d9946a197d80885781e9707c5841
2023-08-03 04:42:30 -07:00