Skip to content

Commit

Permalink
Use host-relative WebSocket URLs when launching new debugger
Browse files Browse the repository at this point in the history
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
  • Loading branch information
motiz88 authored and facebook-github-bot committed Feb 26, 2024
1 parent eb1fc04 commit f86c054
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@ import getDevToolsFrontendUrl from '../utils/getDevToolsFrontendUrl';
describe('getDevToolsFrontendUrl', () => {
const webSocketDebuggerUrl =
'ws://localhost:8081/inspector/debug?device=1a9372c&page=-1';
const devServerUrl = 'http://localhost:8081';

describe('given an absolute devServerUrl', () => {
const devServerUrl = 'http://localhost:8081';
const experiments = {
enableNetworkInspector: false,
enableNewDebugger: false,
enableOpenDebuggerRedirect: false,
};

it('should return a valid url for all experiments off', async () => {
const experiments = {
enableNetworkInspector: false,
enableNewDebugger: false,
enableOpenDebuggerRedirect: false,
};
describe('relative: false (default)', () => {
test('should return a valid url for all experiments off', async () => {
const actual = getDevToolsFrontendUrl(
experiments,
webSocketDebuggerUrl,
Expand All @@ -33,18 +33,13 @@ describe('getDevToolsFrontendUrl', () => {
expect(url.host).toBe('localhost:8081');
expect(url.pathname).toBe('/debugger-frontend/rn_inspector.html');
expect(url.searchParams.get('ws')).toBe(
'localhost:8081/inspector/debug?device=1a9372c&page=-1',
'/inspector/debug?device=1a9372c&page=-1',
);
});

it('should return a valid url for enableNetworkInspector experiment on', async () => {
const experiments = {
enableNetworkInspector: true,
enableNewDebugger: true,
enableOpenDebuggerRedirect: false,
};
test('should return a valid url for enableNetworkInspector experiment on', async () => {
const actual = getDevToolsFrontendUrl(
experiments,
{...experiments, enableNetworkInspector: true, enableNewDebugger: true},
webSocketDebuggerUrl,
devServerUrl,
);
Expand All @@ -53,14 +48,26 @@ describe('getDevToolsFrontendUrl', () => {
expect(url.pathname).toBe('/debugger-frontend/rn_inspector.html');
expect(url.searchParams.get('unstable_enableNetworkPanel')).toBe('true');
expect(url.searchParams.get('ws')).toBe(
'localhost:8081/inspector/debug?device=1a9372c&page=-1',
'/inspector/debug?device=1a9372c&page=-1',
);
});
});

describe('given a relative devServerUrl', () => {
const relativeDevServerUrl = '';
test('should return a full WS URL if on a different host than the dev server', () => {
const otherWebSocketDebuggerUrl =
'ws://localhost:9000/inspector/debug?device=1a9372c&page=-1';
const actual = getDevToolsFrontendUrl(
experiments,
otherWebSocketDebuggerUrl,
devServerUrl,
);
const url = new URL(actual);
expect(url.searchParams.get('ws')).toBe(
'localhost:9000/inspector/debug?device=1a9372c&page=-1',
);
});
});

describe('relative: true', () => {
function assertValidRelativeURL(relativeURL: string): URL {
const anyBaseURL = new URL('https://www.example.com');
try {
Expand All @@ -71,40 +78,53 @@ describe('getDevToolsFrontendUrl', () => {
}
}

it('should return a valid url for all experiments off', async () => {
const experiments = {
enableNetworkInspector: false,
enableNewDebugger: false,
enableOpenDebuggerRedirect: false,
};
test('should return a valid url for all experiments off', async () => {
const actual = getDevToolsFrontendUrl(
experiments,
webSocketDebuggerUrl,
relativeDevServerUrl,
devServerUrl,
{
relative: true,
},
);
const url = assertValidRelativeURL(actual);
expect(url.pathname).toBe('/debugger-frontend/rn_inspector.html');
expect(url.searchParams.get('ws')).toBe(
'localhost:8081/inspector/debug?device=1a9372c&page=-1',
'/inspector/debug?device=1a9372c&page=-1',
);
});

it('should return a valid url for enableNetworkInspector experiment on', async () => {
const experiments = {
enableNetworkInspector: true,
enableNewDebugger: true,
enableOpenDebuggerRedirect: false,
};
test('should return a valid url for enableNetworkInspector experiment on', async () => {
const actual = getDevToolsFrontendUrl(
experiments,
{...experiments, enableNetworkInspector: true, enableNewDebugger: true},
webSocketDebuggerUrl,
relativeDevServerUrl,
devServerUrl,
{
relative: true,
},
);
const url = assertValidRelativeURL(actual);
expect(url.pathname).toBe('/debugger-frontend/rn_inspector.html');
expect(url.searchParams.get('unstable_enableNetworkPanel')).toBe('true');
expect(url.searchParams.get('ws')).toBe(
'localhost:8081/inspector/debug?device=1a9372c&page=-1',
'/inspector/debug?device=1a9372c&page=-1',
);
});

test('should return a full WS URL if on a different host than the dev server', () => {
const otherWebSocketDebuggerUrl =
'ws://localhost:8082/inspector/debug?device=1a9372c&page=-1';
const actual = getDevToolsFrontendUrl(
experiments,
otherWebSocketDebuggerUrl,
devServerUrl,
{
relative: true,
},
);
const url = assertValidRelativeURL(actual);
expect(url.searchParams.get('ws')).toBe(
'localhost:8082/inspector/debug?device=1a9372c&page=-1',
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ export default function openDebuggerMiddleware({
Location: getDevToolsFrontendUrl(
experiments,
target.webSocketDebuggerUrl,
// Use a relative URL.
'',
serverBaseUrl,
{relative: true},
),
});
res.end();
Expand Down
44 changes: 37 additions & 7 deletions packages/dev-middleware/src/utils/getDevToolsFrontendUrl.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,21 @@ export default function getDevToolsFrontendUrl(
experiments: Experiments,
webSocketDebuggerUrl: string,
devServerUrl: string,
options?: $ReadOnly<{
relative?: boolean,
}>,
): string {
const scheme = new URL(webSocketDebuggerUrl).protocol.slice(0, -1);
const webSocketUrlWithoutProtocol = webSocketDebuggerUrl.replace(
/^wss?:\/\//,
'',
);
const appUrl = `${devServerUrl}/debugger-frontend/rn_inspector.html`;
const wsParam = getWsParam({
webSocketDebuggerUrl,
devServerUrl,
});

const appUrl =
(options?.relative === true ? '' : devServerUrl) +
'/debugger-frontend/rn_inspector.html';

const searchParams = new URLSearchParams([
[scheme, webSocketUrlWithoutProtocol],
[wsParam.key, wsParam.value],
['sources.hide_add_folder', 'true'],
]);
if (experiments.enableNetworkInspector) {
Expand All @@ -36,3 +41,28 @@ export default function getDevToolsFrontendUrl(

return appUrl + '?' + searchParams.toString();
}

function getWsParam({
webSocketDebuggerUrl,
devServerUrl,
}: $ReadOnly<{
webSocketDebuggerUrl: string,
devServerUrl: string,
}>): {
key: string,
value: string,
} {
const wsUrl = new URL(webSocketDebuggerUrl);
const serverHost = new URL(devServerUrl).host;
let value;
if (wsUrl.host === serverHost) {
// Use a path-absolute (host-relative) URL
// Depends on https://github.com/facebookexperimental/rn-chrome-devtools-frontend/pull/4
value = wsUrl.pathname + wsUrl.search + wsUrl.hash;
} else {
// Standard URL format accepted by the DevTools frontend
value = wsUrl.host + wsUrl.pathname + wsUrl.search + wsUrl.hash;
}
const key = wsUrl.protocol.slice(0, -1);
return {key, value};
}

0 comments on commit f86c054

Please sign in to comment.