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

fix(sw): fix UI mode on codespaces by not passing server #33664

Merged
merged 5 commits into from
Nov 19, 2024

Conversation

Skn0tt
Copy link
Member

@Skn0tt Skn0tt commented Nov 19, 2024

Yesterday we found that UI mode is broken on codespaces b/c the new server parameter points to localhost, which isn't available when the UI is accessed over Codespace's port forwarding host.

This PR fixes that by making the server parameter only used in HMR mode. I'm also making the web server connection respect the server param in this PR.

I've developed this on a Codespace and verified that it works.

@Skn0tt Skn0tt requested a review from dgozman November 19, 2024 08:40
@Skn0tt Skn0tt self-assigned this Nov 19, 2024

This comment has been minimized.

This comment has been minimized.

@Skn0tt Skn0tt changed the title fix(sw): fix UI mode on codespaces by passing server as relative path fix(sw): fix UI mode on codespaces by not passing server Nov 19, 2024
const guid = searchParams.get('ws');
const wsURL = new URL(`../${guid}`, window.location.toString());
wsURL.protocol = (window.location.protocol === 'https:' ? 'wss:' : 'ws:');
let testServerBaseUrl = new URL('../', window.location.href);
Copy link
Member Author

Choose a reason for hiding this comment

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

window.location.href is something like https://localhost:1234/tracing/uiMode.html, and this expression turns that into https://localhost:1234/. If it was served deeper, like under https://localhost:1234/azdo-subdir/tracing/uiMode.html, then it'd turn into https://localhost:1234/azdo-subdor/.

This comment has been minimized.

@Skn0tt
Copy link
Member Author

Skn0tt commented Nov 19, 2024

@dgozman i've applied the changes we talked about.

Copy link
Contributor

Test results for "tests 1"

1 flaky ⚠️ [playwright-test] › ui-mode-test-watch.spec.ts:145:5 › should watch all @windows-latest-node18-1

36952 passed, 650 skipped
✔️✔️✔️

Merge workflow run.

@Skn0tt Skn0tt merged commit 8c1002a into main Nov 19, 2024
29 checks passed
@Skn0tt Skn0tt deleted the sw-server-relative branch November 19, 2024 15:39
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