-
Notifications
You must be signed in to change notification settings - Fork 907
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
breaking: Drop browser helpers from cli-tools, simplify middleware #2588
Conversation
import {launchEditor} from '@react-native-community/cli-tools'; | ||
|
||
type Options = { | ||
watchFolders: ReadonlyArray<string>; |
There was a problem hiding this comment.
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'
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, thanks for explanation!
05076b8
to
76ad4d6
Compare
.use('/open-url', openURLMiddleware) | ||
.use('/status', statusPageMiddleware) | ||
// TODO: Remove. Requires standardized JSON body parsing support in Metro. |
There was a problem hiding this comment.
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.
Summary:
Completes the removal of pieces related to Remote JS Debugging.
Breaking: The
launchDefaultBrowser
andlaunchDebugger
exports ofcli-tools
are removed.Test Plan:
Repeated test plan of #2587.
✅ For a test error in
RNTester
, component stack is correctly symbolicated in RedBoxChecklist