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

[cli] Fix start hermes debugger crash when starting with --dev-client #19919

Merged
merged 4 commits into from Nov 8, 2022

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Nov 8, 2022

Why

a regression from #19872 which starting dev-server with --dev-client parameter and pressing j to launch hermes debugger. the cli will crash with TypeError: Only HTTP(S) protocols are supported.

How

with --dev-client parameter, the returned getNativeRuntimeUrl will not be scheme://expo-development-client/?url=.... since the url is quite different between expo-go and dev builds. this pr adds a getJsInspectorBaseUrl specific for the inspector server.

  • why not use getDevServerUrl
    getDevServerUrl will return http://localhost by default which breaks on windows. this is what [cli][dev-server] Fix hermes debugger error on windows and linux #19872 originally trying to address.
  • why not use getExpoGoUrl
    getExpoGoUrl is a protected method in the server. even though we could access the internal property by server['getExpoGoUrl']().replace(/^exp:\/\//, 'http://'). that is not clear and not having a typescript typing in BundlerDevServer.

Test Plan

  • ✅ start server with npx expo start and press j
  • ✅ start server with npx expo start --dev-client and press j

Unit Test

 PASS   @expo/cli  src/start/server/__tests__/BundlerDevServer-test.ts
  getJsInspectorBaseUrl
    ✓ should return http based url
    ✓ should return tunnel url
    ✓ should throw error for unsupported bundler

Checklist

@expo-bot expo-bot added the bot: passed checks ExpoBot has nothing to complain about label Nov 8, 2022
@Kudo Kudo marked this pull request as ready for review November 8, 2022 06:47
@brentvatne brentvatne merged commit 57a0d51 into main Nov 8, 2022
@brentvatne brentvatne deleted the @kudo/sdk47/fix-hermes-debugger-with-dev-client branch November 8, 2022 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants