Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove ReactNativeVersionCheck from List of Disabled Files for Flow #9223

Closed
chiaramooney opened this issue Dec 1, 2021 · 3 comments
Closed

Remove ReactNativeVersionCheck from List of Disabled Files for Flow #9223

chiaramooney opened this issue Dec 1, 2021 · 3 comments
Assignees
@chiaramooney
Labels
Area: Infrastructure Deforking enhancement Integration Follow-up
Milestone
0.68

Comments

@chiaramooney
Copy link
Contributor

Creating issue here to track source change during an integration from upstream. ReactNativeVersionCheck.js and ReactNativeVersionCheck.win32.js are throwing errors due to type mismatch for the prerelease parameter following recent change to react-native's prerelease version syntax.
@ghost ghost added the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label Dec 1, 2021
@chrisglein
Copy link
Member

Results from us adding "-preview" in our release names.
I assume there's a patch file in place for our workaround - @chiaramooney can you add links to the integration PR? Sounds like we'd need an upstream change to eliminate the need for a patch here.

@chrisglein chrisglein removed the Needs: Triage 🔍 New issue that needs to be reviewed by the issue management team (label applied by bot) label Jan 3, 2022
@chrisglein chrisglein added this to the 0.68 milestone Jan 3, 2022
@chiaramooney
Copy link
Contributor Author

Edits needed to restore RNW:
https://github.com/chiaramooney/react-native-windows/blob/a260f9733df204b1cc26dd50152142630ebb45b5/packages/%40office-iss/react-native-win32/src/Libraries/Core/ReactNativeVersionCheck.win32.js#L52

https://github.com/chiaramooney/react-native-windows/blob/a260f9733df204b1cc26dd50152142630ebb45b5/vnext/src/Libraries/Core/ReactNativeVersionCheck.windows.js#L1-L57

ReactNativeVersionCheck was already forked for win32. Needed to add a fork for windows. Decision here to be made if we want to add these changes to core.

The reason why RNW breaks by this, but RN doesn't is because they have "null" as the value of prerelease in their ReactNativeVersion.js file, but we have a string value that matches the characters found after the first "-" in the RN version RNW is currently using.

@chiaramooney chiaramooney added the Needs: Attention 👋 An issue that had been tagged "Needs: Author Feedback" has received activity (label applied by bot) label Jan 3, 2022
@chrisglein chrisglein added Deforking and removed Needs: Attention 👋 An issue that had been tagged "Needs: Author Feedback" has received activity (label applied by bot) labels Jan 13, 2022
@chiaramooney chiaramooney mentioned this issue Jan 14, 2022
Closed
facebook-github-bot pushed a commit to facebook/react-native that referenced this issue Jan 26, 2022
@chiaramooney @facebook-github-bot
Update RN Version Expected Structure (#32892)
3d1d4ee 
Summary:
Current syntax options for RN version values break Windows. Following change to nightly build format to be 0.0.0-X-X-X, prerelease value is now a string (X-X-X).

microsoft/react-native-windows#9223

## Changelog

[General] [Fixed] - Fix RN version syntax to match new nightly build structure.

Pull Request resolved: #32892

Reviewed By: cortinico

Differential Revision: D33712950

Pulled By: lunaleaps

fbshipit-source-id: 9e47cae34930ee624a863c832430962354ebb5be
@chiaramooney
Copy link
Contributor Author

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chiaramooney chiaramooney

Labels
Area: Infrastructure Deforking enhancement Integration Follow-up
Projects
None yet
Milestone
0.68
Development

No branches or pull requests
2 participants
@chrisglein @chiaramooney