Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

breaking: Drop browser helpers from cli-tools, simplify middleware #2588

Merged
merged 1 commit into from
Jan 31, 2025

Conversation

huntie
Copy link
Collaborator

@huntie huntie commented Jan 29, 2025

Summary:

Completes the removal of pieces related to Remote JS Debugging.

Breaking: The launchDefaultBrowser and launchDebugger exports of cli-tools are removed.

Test Plan:

Repeated test plan of #2587.

image

✅ For a test error in RNTester, component stack is correctly symbolicated in RedBox

Checklist

  • Documentation is up to date to reflect these changes.
  • Follows commit message convention described in CONTRIBUTING.md

import {launchEditor} from '@react-native-community/cli-tools';

type Options = {
watchFolders: ReadonlyArray<string>;
Copy link
Collaborator Author

@huntie huntie Jan 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@szymonrybczak Relevant to your other comment, watchFolders is staying around at this level because it ought to (not currently) be used to parse out substitutions from URLs sent by the debugger, e.g. '[metro-project]/path/to/file.js'.

cc @hoxyq @robhogan

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, thanks for explanation!

@huntie huntie force-pushed the simplify-open-middleware branch from 05076b8 to 76ad4d6 Compare January 29, 2025 16:09
.use('/open-url', openURLMiddleware)
.use('/status', statusPageMiddleware)
// TODO: Remove. Requires standardized JSON body parsing support in Metro.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fun 😄. We have a fix to make around this in Metro. See the original CLI PR: #1147.

I’ve actioned an initial change in React Native — also required for the substitution of body-parser for the endpoints in this PR:

Once removed, we can simplify further and apply .use(json()) against this entire middleware stack.

@huntie huntie requested a review from szymonrybczak January 31, 2025 11:19
@huntie huntie merged commit 2143752 into react-native-community:main Jan 31, 2025
8 checks passed
@huntie huntie deleted the simplify-open-middleware branch January 31, 2025 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants