Page MenuHomePhabricator

[native] Make 'natDevHostname' configurable from env
ClosedPublic

Authored by bartek on Jan 18 2023, 1:03 AM.
Tags
None
Referenced Files
F3245894: D6293.diff
Thu, Nov 14, 8:39 PM
F3245730: D6293.id21035.diff
Thu, Nov 14, 8:14 PM
Unknown Object (File)
Fri, Nov 8, 10:10 AM
Unknown Object (File)
Fri, Nov 8, 10:10 AM
Unknown Object (File)
Fri, Nov 8, 10:10 AM
Unknown Object (File)
Thu, Nov 7, 11:37 PM
Unknown Object (File)
Tue, Oct 29, 1:47 AM
Unknown Object (File)
Thu, Oct 17, 6:37 AM
Subscribers

Details

Summary

Resolves concerns raised in ENG-2734 and ENG-2707.

This diff introduces two environment variables, whose are read by Expo start command (yarn dev):

  • COMM_DEV - This one must be set, otherwise natDevHostname will be null. Its purpose is not to embed developer machine IP in production builds.
  • COMM_NAT_DEV_HOSTNAME - This one can be used to overwrite the autodetected IP address - I consider this as a replacement for network.json in special cases we want to have full control over the hostname.

Depends on D6267

Test Plan

The expo config command below evaluates the app.config.js (and some other Expo defaults), it's used internally when running Metro dev server.

cd native

# should be empty
yarn expo config | grep natDevHostname

# should detect local IP
COMM_DEV=1 yarn expo config | grep natDevHostname

# should resolve to '1.2.3.4'
COMM_DEV=1 COMM_NAT_DEV_HOSTNAME=1.2.3.4 yarn expo config | grep natDevHostname

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Jan 18 2023, 1:19 AM
bartek edited the summary of this revision. (Show Details)

This solution makes sense to me!

This revision is now accepted and ready to land.Jan 18 2023, 6:17 PM
This revision was landed with ongoing or failed builds.Jan 19 2023, 12:03 AM
This revision was automatically updated to reflect the committed changes.