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

Support path-only WebSocket URLs in hosted mode #4

Merged
merged 1 commit into from
Feb 23, 2024

Conversation

motiz88
Copy link

@motiz88 motiz88 commented Feb 23, 2024

Summary

React Native currently uses the DevTools frontend in hosted mode, and specifically hosts the frontend on the same HTTP(S) server as the CDP WebSocket. Internally at Meta, that server is often running on a remote machine, so the need comes up to set up port tunnels and rewrite URLs accordingly.

To simplify the rewriting logic, it would be helpful to not have to repeat the host:port information inside the ws / wss query param, and instead have DevTools interpret the parameter as a path-absolute URL that's relative to the hosted mode server. We implement this here in a backwards-compatible way that could in theory be upstreamed to devtools-frontend.

Test plan

  1. Build and run in hosted mode
  2. Open DevTools-on-DevTools
  3. Manually pass different URL formats via ws and wss and observe (via error messages in the console) that DevTools is trying to connect to the expected full WebSocket URLs.

Upstreaming plan

  • This commit should be sent as a patch to the upstream devtools-frontend repo. I've reviewed the contribution guide.
  • This commit is React Native-specific and cannot be upstreamed.

Copy link

@huntie huntie left a comment

Choose a reason for hiding this comment

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

LGTM :)

Can I double check the Git submodule updates?

motiz88 added a commit to motiz88/react-native that referenced this pull request Feb 23, 2024
motiz88 added a commit to motiz88/react-native that referenced this pull request Feb 23, 2024
Summary:
Changelog: [Internal]

Uses the capability introduced in facebookexperimental/rn-chrome-devtools-frontend#4 to avoid repeating the dev server's host:port in the `ws` / `wss` parameter we pass to the Chrome DevTools frontend. This gives us more flexibility to handle port forwarding and redirects outside of `dev-middleware`. This is mostly useful in Meta's internal VS Code remoting setup, but this particular change should work equally well in open source.

Differential Revision: D54107316
@motiz88
Copy link
Author

motiz88 commented Feb 23, 2024

the Git submodule updates

the what

Yeah lemme undo that

@motiz88 motiz88 force-pushed the support-path-only-ws-url branch from 879f4f7 to 7352557 Compare February 23, 2024 11:11
@motiz88 motiz88 merged commit 9ceb0ad into facebookexperimental:main Feb 23, 2024
2 checks passed
@motiz88 motiz88 deleted the support-path-only-ws-url branch February 23, 2024 11:15
motiz88 added a commit to motiz88/react-native that referenced this pull request Feb 26, 2024
Summary:
Changelog: [Internal]

Uses the capability introduced in facebookexperimental/rn-chrome-devtools-frontend#4 to avoid repeating the dev server's host:port in the `ws` / `wss` parameter we pass to the Chrome DevTools frontend. This gives us more flexibility to handle port forwarding and redirects outside of `dev-middleware`. This is mostly useful in Meta's internal VS Code remoting setup, but this particular change should work equally well in open source.

Reviewed By: huntie

Differential Revision: D54107316
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Feb 26, 2024
Summary:
Changelog: [Internal]

Uses the capability introduced in facebookexperimental/rn-chrome-devtools-frontend#4 to avoid repeating the dev server's host:port in the `ws` / `wss` parameter we pass to the Chrome DevTools frontend. This gives us more flexibility to handle port forwarding and redirects outside of `dev-middleware`. This is mostly useful in Meta's internal VS Code remoting setup, but this particular change should work equally well in open source.

Reviewed By: huntie

Differential Revision: D54107316

fbshipit-source-id: 68d4dbf4849ca431274bfb0dc8a4e05981bdd5b5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants