Fix custom port listening configuration through metro.config.js

[afonsograca]

Summary:

After updating my project to 0.73.2 I noticed that even though I had a specific port set in my metro.config.js, every time I'd start my project, it was running on port 8081. Passing the --port argument would allow me to change the port, but the config from metro did not. I checked if the metro config was being properly applied, using --verbose and it was.

So I dug a bit, trying to figure out what had changed and noticed the coalescing of the value, whenever the argument --port is not present. That seemed odd since it meant that there's always a port defined for the options of loadMetroConfig, which would always be used in the loadConfig step.

To confirm I was on the right track I went to the cli-plugin-metro repo, to the last release before the move here, and noticed that there was no coalescing in the same method.

In this PR, I remove the coalescing of the port from runServer.js from the community-cli-plugin, to allow the port configuration through metro.config.js.

Changelog:

[INTERNAL] [FIXED] - Fix server port configuration via metro.config.js

Test Plan:

Running yarn start and verifying that:

• it would listen to port 8081 if no argument nor a custom port was set in metro.config.js
• it would listen to port 8082 if that one was defined in metro.config.js
• it would listend to port 8083 if that port was passed as an argument to the command (i.e. yarn start --port 8083 even though port 8082 was defined in metro.config.js

[robhogan]

Is the = 8081 here necessary? metroConfig should be fully hydrated at this point and populated with the upstream Metro defaults from metro-config.

[afonsograca]

I don't think so, but I don't have the context as to why the coalescing was added in the first place so I wanted to make sure we had a failsafe. Happy to remove it if you don't think it's necessary. the less the better.

[robhogan]

Ah, that might be because Metro's default used to be 8080 whereas RN's was 8081 - CC @huntie who might recall the context there.

Now that Metro and RN are aligned you're right IMO - the less repetition of this value, the better - let's remove it.

[afonsograca]

updated!

[afonsograca]

it seems something failed, but I don't have permissions to see what it is. could you let me know?

[robhogan]

This is a great catch @afonsograca - many thanks!

[facebook-github-bot]

@robhogan has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@robhogan merged this pull request in 0950916.

[github-actions]

This pull request was successfully merged by @afonsograca in 0950916.

^{When will my fix make it into a release? | How to file a pick request?}