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

Authored by bartek on Jan 12 2023, 2:04 PM.

Details

Reviewers

ashoat

Commits

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

Summary

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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bartek created this revision.Jan 12 2023, 2:04 PM

bartek held this revision as a draft.

Herald added subscribers: atul, tomek. · View Herald TranscriptJan 12 2023, 2:04 PM

bartek edited the summary of this revision. (Show Details)Jan 12 2023, 2:13 PM

bartek edited the test plan for this revision. (Show Details)

bartek edited the test plan for this revision. (Show Details)Jan 12 2023, 2:15 PM

bartek attached a referenced file: F328509: RPReplay_Final1673559951.mov. (Show Details)

bartek attached a referenced file: F328503: Screenshot 2023-01-12 at 22.48.15.png. (Show Details)

bartek edited the test plan for this revision. (Show Details)Jan 12 2023, 2:18 PM

Harbormaster failed remote builds in B15327: Diff 20903!Jan 12 2023, 2:19 PM

bartek edited the test plan for this revision. (Show Details)Jan 12 2023, 2:21 PM

bartek attached a referenced file: F328519: rpreplay-final1673559951_2zCEOKtT.mp4. (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

In D6257#187503, @ashoat wrote:

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