Add serverBaseUrl option, set client-accessible URL value externally #39456
Conversation
This file contains 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
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D49276125 |
huntie
added a commit
to huntie/react-native
that referenced
this pull request
Sep 15, 2023
…acebook#39456) Summary: **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] Differential Revision: D49276125
huntie
force-pushed
the
export-D49276125
branch
from
312c446 to
02aaaea
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49276125 |
Base commit: 88395bb |
|
This pull request was exported from Phabricator. Differential Revision: D49276125 |
huntie
added a commit
to huntie/react-native
that referenced
this pull request
Sep 15, 2023
…acebook#39456) Summary: **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
huntie
force-pushed
the
export-D49276125
branch
from
02aaaea to
e791650
Compare
huntie
added a commit
to huntie/react-native
that referenced
this pull request
Sep 15, 2023
…acebook#39456) Summary: **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
huntie
force-pushed
the
export-D49276125
branch
from
e791650 to
4435d77
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49276125 |
huntie
added a commit
to huntie/react-native
that referenced
this pull request
Sep 15, 2023
…acebook#39456) Summary: **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
|
This pull request was exported from Phabricator. Differential Revision: D49276125 |
huntie
force-pushed
the
export-D49276125
branch
from
4435d77 to
d48c8b9
Compare
huntie
added a commit
to huntie/react-native
that referenced
this pull request
Sep 15, 2023
…acebook#39456) Summary: **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
huntie
force-pushed
the
export-D49276125
branch
from
d48c8b9 to
4bb3222
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49276125 |
…acebook#39456) Summary: **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
huntie
force-pushed
the
export-D49276125
branch
from
4bb3222 to
1d06f38
Compare
|
This pull request was exported from Phabricator. Differential Revision: D49276125 |
|
This pull request was successfully merged by @huntie in 850e550. When will my fix make it into a release? | Upcoming Releases |
facebook-github-bot
pushed a commit
that referenced
this pull request
Nov 15, 2024
…ist' by requestor Summary: `serverBaseUrl` is currently documented as: > The base URL to the dev server, as addressible from the local developer machine This is problematic in general because `dev-middleware` on a server doesn't necessarily know about where clients might be reaching it from, how tunnels or port-forwards are set up, etc., and this can change over the lifetime of the server and vary between clients. Indeed, our own use of `serverBaseUrl` from both `community-cli-plugin` and internally simply sets it to the host and port the dev server is listening on - ie it's the address of the dev server accessible *from the server*. This PR changes the docs, redefining `serverBaseUrl`, to match the way we currently specify it. One usage where we *do* want the previously documented behaviour is in responses to `/json/list` (`getPageDescriptions`) where the URLs in the response should be reachable by a browser requesting `/json/list`. Here, we use the request (host header, etc.) to attempt to get working base URL. History: It should be mentioned that this is the latest in a series of changes like this: - #39394 - #39456 Learning from those: - This change does *not* break Android emulators, which routes `10.0.2.2` to localhost, or other routed devices, because `/open-debugger` still uses server-relative URLs, and now formally delegates to `BrowserLauncher` to decide what to do with those URLs (internally, VSCode / `xdg-open` handles port forwarding) - Middleware configuration is no longer required to specify how it is reachable from clients. This sets up some subsequent changes for more robust handling of tunnelled connections. Changelog: [General][Breaking] dev-middleware: Frameworks should specify `serverBaseUrl` relative to the middleware host. Differential Revision: D65974487
facebook-github-bot
pushed a commit
that referenced
this pull request
Nov 17, 2024
…ist' by requestor (#47628) Summary: `serverBaseUrl` is currently documented as: > The base URL to the dev server, as addressible from the local developer machine This is problematic in general because `dev-middleware` on a server doesn't necessarily know about where clients might be reaching it from, how tunnels or port-forwards are set up, etc., and this can change over the lifetime of the server and vary between clients. Indeed, our own use of `serverBaseUrl` from both `community-cli-plugin` and internally simply sets it to the host and port the dev server is listening on - ie it's the address of the dev server accessible *from the server*. This PR changes the docs, redefining `serverBaseUrl`, to match the way we currently specify it. One usage where we *do* want the previously documented behaviour is in responses to `/json/list` (`getPageDescriptions`) where the URLs in the response should be reachable by a browser requesting `/json/list`. Here, we use the request (host header, etc.) to attempt to get working base URL. History: It should be mentioned that this is the latest in a series of changes like this: - #39394 - #39456 Learning from those: - This change does *not* break Android emulators, which routes `10.0.2.2` to localhost, or other routed devices, because `/open-debugger` still uses server-relative URLs, and now formally delegates to `BrowserLauncher` to decide what to do with those URLs (internally, VSCode / `xdg-open` handles port forwarding) - Middleware configuration is no longer required to specify how it is reachable from clients. This sets up some subsequent changes for more robust handling of tunnelled connections. Changelog: [General][Breaking] dev-middleware: Frameworks should specify `serverBaseUrl` relative to the middleware host. Differential Revision: D65974487
facebook-github-bot
pushed a commit
that referenced
this pull request
Nov 18, 2024
…ist' by requestor (#47628) Summary: Pull Request resolved: #47628 `serverBaseUrl` is currently documented as: > The base URL to the dev server, as addressible from the local developer machine This is problematic in general because `dev-middleware` on a server doesn't necessarily know about where clients might be reaching it from, how tunnels or port-forwards are set up, etc., and this can change over the lifetime of the server and vary between clients. Indeed, our own use of `serverBaseUrl` from both `community-cli-plugin` and internally simply sets it to the host and port the dev server is listening on - ie it's the address of the dev server accessible *from the server*. This PR changes the docs, redefining `serverBaseUrl`, to match the way we currently specify it. One usage where we *do* want the previously documented behaviour is in responses to `/json/list` (`getPageDescriptions`) where the URLs in the response should be reachable by a browser requesting `/json/list`. Here, we use the request (host header, etc.) to attempt to get working base URL. History: It should be mentioned that this is the latest in a series of changes like this: - #39394 - #39456 Learning from those: - This change does *not* break Android emulators, which routes `10.0.2.2` to localhost, or other routed devices, because `/open-debugger` still uses server-relative URLs, and now formally delegates to `BrowserLauncher` to decide what to do with those URLs (internally, VSCode / `xdg-open` handles port forwarding) - Middleware configuration is no longer required to specify how it is reachable from clients. This sets up some subsequent changes for more robust handling of tunnelled connections. Changelog: [General][Breaking] dev-middleware: Frameworks should specify `serverBaseUrl` relative to the middleware host. Reviewed By: huntie Differential Revision: D65974487 fbshipit-source-id: 1face8fc7715df387f75b329e80932d8543ee419
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary:
Fixes new debugger launch flow on Android:
D49158227 (#39394) aimed to improve proxy-safe behaviour for remote dev servers by auto-detecting the appropriate server URL for clients using the
Hostheader (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.2for the stock Android emulator.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
serverBaseUrloption.Changes
serverBaseUrloption tocreateDevMiddleware, designed to be the base URL value for constructing dev server URLs returned in endpoints such as/json/list.localhostcase (now enablinghttps://URLs), but enables remote dev server setups to specify this cleanly.community-cli-plugin.Changelog: [Internal]
Differential Revision: D49276125