Fix RNTester on Windows

[gengjiawen]

Motivation

Currrent, RNTester can not work on windows.

• FixesRNTester-Windows-Android #19654.

Test Plan:

pass all current ci.

Release Notes:

[GENERAL] [INTERNAL] [RNTester] - fix RNTester on windows.

[pvdz]

Hi! Thanks for the PR.

Please split the PR up as it is doing two distinct things under the umbrella of one PR, then consider the following;

• Removing the : string flow type annotations

This was not an actual fix to #19654, just a workaround for these files.

The actual fix was to update the file path logic in metro-babel-register. Babel inadvertently changed behavior to no longer normalize paths between posix/windows and Metro was unaware of it. As a result the babel-register calls failed and led to syntax errors on windows. This has been fixed since so this change is no longer required at all.

• Removing packager.sh

This seems relevant to me so it's not entirely clear why you're dropping it. If it's breaking something in Windows please describe what and why dropping this script fixes it. I'm not convinced removing it is the way to go but I could be wrong. For example, you're removing the max-workers command. Wouldn't that make everything run "in band" (so on a single core)? That would be a severe regression.

If you feel this should still be dropped feel free to make a PR for it such that somebody more familiar with this area can take a look.

Thanks

[gengjiawen]

I can split the pr, they are in same pr because I want to fix build problem on windows, which still exists (see #19654).

As for packager.sh, it has to be removed if you want to make it on windows. As for max-workers, I think it can add back.

[gengjiawen]

@pvdz The metro issue has been around for a month. Can you help on this issue ?

[gengjiawen]

@hramos This can be merged.

[gengjiawen]

Some clarification:

• I remove types in some file simply because related file or not flow file.
• My main fix is refactor current unix-like bash to nodejs script so we can test RNTester on windows.

cc @kelset@hramos@janicduplessis

[hramos]

@gengjiawen I'll defer to @pvdz for approval + merging.

[gengjiawen]

@pvdz Can you merge this pr ? I add some clarification in previous comment.

[pvdz]

@gengjiawen Can you confirm this is still an issue on Metro 42.2 (depending on a new version of Babel) and RN 0.57 (which should resolve most pending Windows issues)? The error reported in #19654 was almost certainly caused by the Babel regression that has been fixed by now.

As for this PR, it still drops typing that we want to keep and it still drops a shell script we want to keep so those points have not been addressed.

Please confirm this issue is still blocking you with the latest metro/rn or close this ticket if that's now resolved. Thank you!

[gengjiawen]

The type thing is fixed by metro, I have confirmed here @kelset. As I stated in clarification, My main fix is refactor current unix-like bash to nodejs script so we can test RNTester on windows.

[gengjiawen]

@dulmandakh@janicduplessis Can you guys review my pr ? Thanks.

[gengjiawen]

@rafeca I have updated the code.

[rafeca]

This looks good to me, I'm going to import this to Phabricator.

@hramos : any thoughts?

[facebook-github-bot]

rafeca has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[react-native-bot]

This pull request was closed by @gengjiawen in a286c0e.

Once this commit is added to a release, you will see the corresponding version tag below the description at a286c0e. If the commit has a single master tag, it is not yet part of a release.