Page MenuHomePhabricator

[native] Patch @expo/cli to display dev-server IP
ClosedPublic

Authored by bartek on Jan 12 2023, 2:04 PM.
Tags
None
Referenced Files
F3498040: D6257.diff
Thu, Dec 19, 8:36 PM
Unknown Object (File)
Nov 12 2024, 1:23 AM
Unknown Object (File)
Nov 8 2024, 10:04 AM
Unknown Object (File)
Nov 8 2024, 10:04 AM
Unknown Object (File)
Nov 8 2024, 10:04 AM
Unknown Object (File)
Nov 8 2024, 10:03 AM
Unknown Object (File)
Nov 8 2024, 6:54 AM
Unknown Object (File)
Oct 28 2024, 10:58 PM
Subscribers

Details

Summary

Related: https://linear.app/comm/issue/ENG-2705/expo-cli-qr-code-doesnt-use-the-right-lan-ip-address

Because we run expo start with the --dev-client flag, but we don't have defined a custom Linking URL Scheme, Expo CLI defaults to localhost.

This diff patches @expo/cli and overrides this behavior.

  • Try to autodetect the IP using some utility from the package
  • If failed, try to read the IP from native/facts/network.json
  • If we fail again (require fails or natDevHostname doesn't exist), leave "localhost"

On the other hand, maybe it's worth stopping relying on network.json, as stated in https://linear.app/comm/issue/ENG-2707/replace-nativefactsnetworkjson-with-code-that-detects-lan-ip-address

Test Plan
cd native
yarn dev

The IP should display instead of http://localhost:8081 and QR code should be scannable (it should open some Expo/Metro-specific JSON in the browser)

Screenshot 2023-01-12 at 22.48.15.png (964×1 px, 86 KB)

Diff Detail

Repository
rCOMM Comm
Branch
barthap/expo-cli-local-ip
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek held this revision as a draft.
bartek edited the test plan for this revision. (Show Details)
bartek published this revision for review.Jan 12 2023, 2:27 PM

Seems hacky, wonder if we should do ENG-2706 or ENG-2707 instead. But I guess those might take longer and we have this now

This revision is now accepted and ready to land.Jan 12 2023, 2:38 PM

Seems hacky, wonder if we should do ENG-2706 or ENG-2707 instead. But I guess those might take longer and we have this now

Yes, this is a temporary solution, because ENG-2706 requires much more work.

  • I reversed the detection order, because we want to get rid of facts/network.json:
    1. First try to autodetect IP
    2. Fall back to natDevHostname
  • Added comments
This revision was landed with ongoing or failed builds.Jan 13 2023, 12:58 AM
This revision was automatically updated to reflect the committed changes.