Upgrade to 0.62 #4587

NickGerleman · 2020-04-14T12:28:28Z

Squashed version of #4300

This upgrades us to 0.62.2. Comments were left on individual interesting parts, but some of the challenges were around increasing flow-strictness, requiring us to replace some of our stubs. I've opted to try to align JS as close as possible to some upstream variant for sake of making future merges easier.

This works correctly with basic scenarios in RNTester for both React Native Windows and React Native Win32. There are a number of known issues that will be addressed as followup:

Microsoft Reviewers: Open in CodeFlow

NickGerleman · 2020-04-14T12:35:25Z

packages/override-tools/src/Cli.ts

@@ -188,8 +188,6 @@ async function addOverride(overridePath: string) {
 * Remove an override from the manifest
 */
 async function removeOverride(overridePath: string) {
-  await checkFileExists('override', overridePath);


checkFileExists [](start = 8, length = 15)

Removed this so that we can delete overrides from the manifest easily if the actual file goes away

NickGerleman · 2020-04-14T12:37:10Z

packages/override-tools/src/Cli.ts

    console.error(chalk.red(errorMessage));
    overridesMissing.forEach(err => console.error(` - ${err.file}`));
    console.error();
  }

  if (baseFilesNotFound.length > 0) {
    const errorMessage =
-      "Found overrides whose original files do not exist. Remove existing overrides using 'yarn override remove <override>':";
+      "Found overrides whose original files do not exist. Remove existing overrides using 'yarn override remove <override>' (where override is package relative):";


relative [](start = 151, length = 8)

So, this is kind of annoying. yarn will lie about cwd and always pretend you're running the script from the package directory. npx doesn't run into this, but will also go download the override npm package if one isn't found, and we've seen issues before where yarn doesn't link the override bin correctly without a force or clean install,.

NickGerleman · 2020-04-14T12:37:28Z

packages/override-tools/src/Manifest.ts

@@ -81,7 +81,7 @@ export default class Manifest {
        const baseFile = override.baseFile;
        const baseContent = await this.reactRepo.getFileContents(baseFile);
        if (baseContent === null) {
-          errors.push({type: 'baseFileNotFound', file: override.baseFile});
+          errors.push({type: 'baseFileNotFound', file: override.file});


file [](start = 64, length = 4)

Small change for consistency

NickGerleman · 2020-04-14T12:38:11Z

packages/react-native-win32/.flowconfig

@@ -39,10 +46,6 @@
 <PROJECT_ROOT>/RNTester/js/components/ListExampleShared.js
 <PROJECT_ROOT>/RNTester/js/components/RNTesterExampleFilter.js

-; These examples currently uses mac dynamic colors
-<PROJECT_ROOT>/RNTester/js/ActivityIndicatorExample.js


DarkModeExample doesn't exist in the repo and the top doesn't seem to have issues? Removed the suppression.

NickGerleman · 2020-04-14T12:39:02Z

packages/react-native-win32/.flowconfig

-; Ignore duplicate module providers
-; For RN Apps installed via npm, "Libraries" folder is inside
-; "node_modules/react-native" but in the source repo it is in the root
-.*/Libraries/react-native/React.js


native [](start = 19, length = 6)

the react-native dir got nuked, but React.js didn't exist in there anyway? Cleaned this up

NickGerleman · 2020-04-14T12:39:51Z

packages/react-native-win32/.flowconfig

@@ -115,34 +113,10 @@ module.file_ext=.js
 module.file_ext=.json
 module.file_ext=.win32.js

-module.system=haste


system [](start = 7, length = 6)

We should have gotten rid of this in the 61 timeframe, since we no longer use Haste and this can lead to flow following the wrong requires

NickGerleman · 2020-04-14T12:40:32Z

packages/react-native-win32/.flowconfig

@@ -153,8 +127,14 @@ suppress_type=$FlowFixMeEmpty

 suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe\\($\\|[^(]\\|(\\(<VERSION>\\)? *\\(site=[a-z,_]*react_native\\(_ios\\)?_\\(oss\\|fb\\)[a-z,_]*\\)?)\\)
 suppress_comment=\\(.\\|\n\\)*\\$FlowIssue\\((\\(<VERSION>\\)? *\\(site=[a-z,_]*react_native\\(_ios\\)?_\\(oss\\|fb\\)[a-z,_]*\\)?)\\)?:? #[0-9]+
+suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe\\($\\|[^(]\\|(\\(<VERSION>\\)? *\\(site=[a-z,_]*react_native\\(_android\\)?_\\(oss\\|fb\\)[a-z,_]*\\)?)\\)


suppress_comment [](start = 0, length = 16)

We derive from both Android and iOS files, so I added the Android specific flow suppressions in addition to the existing iOS ones.

NickGerleman · 2020-04-14T12:41:02Z

packages/react-native-win32/.gitignore

 /IntegrationTests
 /jest
 /lib
 /Libraries
 /RNTester
 /RNTester.*
 /temp
-/WorkingHeaders


WorkingHeaders [](start = 1, length = 14)

This has been gone long enough that I should be able to clean it up now without annoying everyone

NickGerleman · 2020-04-14T12:41:25Z

packages/react-native-win32/.gitignore

@@ -1,12 +1,12 @@
 /dist
 /flow
-/flow-typed


flow-typed [](start = 1, length = 10)

We haven't copied this in a while, since it's not part of the published RN package

NickGerleman · 2020-04-14T12:42:18Z

packages/react-native-win32/just-task.js

@@ -70,8 +70,8 @@ task('ts', () => {
 });
 task('clean', () => {
  return cleanTask(
-    ['dist', 'flow', 'flow-typed', 'jest', 'Libraries', 'RNTester', 'lib'].map(


flow [](start = 22, length = 4)

We haven't been doing anything with flow-typed or lib for quite a while, as neither have existed since we're deforked at the very least. Cleaned these up.

NickGerleman · 2020-04-14T12:43:24Z

packages/react-native-win32/package.json

-  "main": "./Libraries/react-native/react-native-implementation.win32.js",
-  "typings": "./Libraries/react-native/typings-main.d.ts",
+  "main": "./index.win32.js",
+  "typings": "./typings-index.d.ts",


index [](start = 24, length = 5)

Renamed from typings-main to typings-index for consistency with RNW. Though I'm not sure why we didn't just go with index.d.ts

NickGerleman · 2020-04-14T12:44:26Z

packages/react-native-win32/src/overrides.json

    },
    {
      "type": "patch",
      "file": "Libraries\\Alert\\Alert.win32.js",
      "baseFile": "Libraries\\Alert\\Alert.js",
-      "baseVersion": "0.61.5",
-      "baseHash": "dd77ba5b6da36da0aea70e638bf6bc2f88e4d073",
+      "baseVersion": "0.62.0-rc.3",


0.62.0-rc.3 [](start = 22, length = 11)

It's totally fine and expected that version numbers are all over the place. This is just the version where I first upgraded a file, and there hasn't been a change since.

NickGerleman · 2020-04-14T12:48:09Z

...react-native-win32/src/Libraries/Components/StatusBar/NativeStatusBarManagerAndroid.win32.js

+  +setHidden: (hidden: boolean) => void;
+}
+
+// [Win32 Change from getEnforcing to get and provide a stub (See #4363)


stub [](start = 56, length = 4)

Stuff including StatusBar became flow-strict, so I added back the real version which will seemingly noop if not Android or iOS already. It does leave the issue of its imports grabbing a module at require-time, so we need to patch this.

We already stub out the native module on Windows (er, techincally a valid implementation) and we should just do that in Office.

Do we have issues anywhere tracking the work we should do in Office? This one seems like one we could add now, before we consume 62?

It's in the comment 😄 See #4363

Tracking Win32 stuff that goes in devmain is a bit funky, but seems okay since the JS is in this repo.

NickGerleman · 2020-04-14T12:48:57Z

packages/react-native-win32/src/Libraries/Components/TextInput/TextInput.Types.win32.ts

@@ -66,10 +66,3 @@ export type IEditingEvent = NativeSyntheticEvent<Readonly<IEditingPayload>>;

 // TODO: Why do I need this one
 export type IPasswordRules = string;
-
-// TODO: Can I get away without this?


? [](start = 36, length = 1)

Yes we can :)

This was only being used for a template parameter for the ObjectRef, and seemingly not being used correctly to begin with.

NickGerleman · 2020-04-14T12:51:19Z

packages/react-native-win32/src/Libraries/Components/Touchable/TouchableBounce.win32.js

+'use strict';
+
+// [Win32 Remove .js
+import Pressability from '../../Pressability/Pressability';


There are a bunch of files like this where we previously .had ".js" in the import path, which forced Metro to load the shared JS even if we had a Windows version. I fixed this upstream, but needed to add a lot of patching for this release.

NickGerleman · 2020-04-14T12:55:55Z

packages/react-native-win32/src/Libraries/Image/NativeImageLoaderWin32.js

+  // Win32]
+
+  // [Win32 These aren't actually implemented, and will just blow up if called
+  // currently.


currently [](start = 5, length = 9)

😢

NickGerleman · 2020-04-14T12:58:21Z

packages/react-native-win32/src/Libraries/Network/RCTNetworking.win32.js

@@ -0,0 +1,96 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.


The TS we had before was just this but missing a couple changes. I replaced this with the Android version with changes to native modules (didn't move from batched bridge to TurboModule syntax yet though)

NickGerleman · 2020-04-14T13:06:45Z

vnext/.flowconfig

 <PROJECT_ROOT>/Libraries/Components/Button.js
 <PROJECT_ROOT>/Libraries/Components/DatePicker/DatePicker.js
-<PROJECT_ROOT>/Libraries/Components/Flyout/Flyout.js
-<PROJECT_ROOT>/Libraries/Components/Glyph/Glyph.js


Glyph [](start = 42, length = 5)

We forgot to clean these up when we removed the non-Windows stubs

NickGerleman · 2020-04-14T13:08:06Z

vnext/DeforkingPatches/.clang-format

@@ -1,91 +1,4 @@
 ---


We previously had a no-op clang-format in the Yoga dir, and this real one in the top level dir. I moved the no-op one to the top level since we discovered FB doesn't CI enforce formatting.

NickGerleman · 2020-04-14T13:12:49Z

vnext/DeforkingPatches/ReactCommon/yoga/yoga/Yoga.cpp

@@ -765,126 +822,141 @@ float YGNodeStyleGetBorder(const YGNodeConstRef node, const YGEdge edge) {
 // Yoga specific properties, not compatible with flexbox specification

 // TODO(T26792433): Change the API to accept YGFloatOptional.
-float YGNodeStyleGetAspectRatio(const YGNodeConstRef node) {
+YOGA_EXPORT float YGNodeStyleGetAspectRatio(const YGNodeConstRef node) {


YOGA_EXPORT [](start = 0, length = 11)

FYI @vmoroz, if we need ABI-safe Yoga we can seemingly set this macro and add a calling convention now. I think the actual APIs might already only be using C style structs.

NickGerleman · 2020-04-14T13:17:34Z

vnext/Desktop.IntegrationTests/DesktopTestRunner.cpp

+          nativeQueue),
+      make_tuple(
+          "StatusBarManager",
+          []() -> unique_ptr<CxxModule> { return make_unique<StubNativeModule>("StatusBarManager"); },


StatusBarManager [](start = 80, length = 16)

ReactUwp/Microsoft.ReactNative have a stub version of this we should consider moving to ReactWindowCore, but this is all going to change and I have reworking Windesktop based tests on my plate to do after we're continually on master anyway.

NickGerleman · 2020-04-14T13:20:47Z

vnext/Desktop.IntegrationTests/WebSocketJSExecutorIntegrationTest.cpp

@@ -51,6 +51,7 @@ TEST_CLASS (WebSocketJSExecutorIntegrationTest) {
  }

  BEGIN_TEST_METHOD_ATTRIBUTE(LoadApplicationScriptSucceeds)
+  TEST_IGNORE()


TEST_IGNORE [](start = 2, length = 11)

@acoates-ms believed we weren't providing enough to actually load a bundle and disabled this.

NickGerleman · 2020-04-14T13:21:50Z

vnext/IntegrationTests/DummyTest.js

-
-'use strict';
-
-const React = require('react');


const [](start = 0, length = 5)

Files in this directory are part of the output directory but were accidentally checked in previously, even though they're now under .gitignore.

NickGerleman · 2020-04-14T13:23:05Z

vnext/ReactCopies/overrides.json

      "issue": 4054
    },
    {
      "type": "copy",
      "file": "IntegrationTests\\IntegrationTestHarnessTest.js",
      "baseFile": "IntegrationTests\\IntegrationTestHarnessTest.js",
-      "baseVersion": "0.61.5",
+      "baseVersion": "0.62.0-rc.3",


baseVersion [](start = 7, length = 11)

A bunch of files (like this one) didn't actually change. It was easier to just dump a new copy of stuff in though and regenerate the manifest.

NickGerleman · 2020-04-14T13:24:18Z

vnext/ReactWindowsCore/Modules/PlatformConstantsModule.cpp

@@ -38,7 +38,7 @@ std::map<std::string, folly::dynamic> PlatformConstantsModule::getConstants() {
      // Since we're out-of-tree, we don't know the exact version of React Native
      // we're paired with. Provide something sane for now, and try to provide a
      // better source of truth later. Tracked by Issue #4073


4073 [](start = 57, length = 4)

I made a fix upstream that will let us use real version that we should get during the next upgrade.

NickGerleman · 2020-04-14T13:26:56Z

vnext/src/Libraries/AppTheme/AppTheme.ts

@@ -5,6 +5,7 @@
 */
 'use strict';


strict [](start = 5, length = 6)

So, this isn't directly related to 0.62, but I discovered when cleaning up our flowconfig that we forgot to remove the stub variant of AppTheme when we removed the stubs of other Windows specific JS. Fixed that here and removed the stub.

NickGerleman · 2020-04-14T13:30:15Z

vnext/src/Libraries/Components/AccessibilityInfo/AccessibilityInfo.windesktop.js


-var TOUCH_EXPLORATION_EVENT = 'touchExplorationDidChange';
+import NativeAccessibilityInfo from './NativeAccessibilityInfo';


NativeAccessibilityInfo [](start = 7, length = 23)

This is now included in a flow-strict file, meaning our typings needed to match. The Android variant of this (and I think iOS) will guard against a missing native module already, so I picked that up verbatim and logged an item to try to share this between platforms.

NickGerleman · 2020-04-14T13:30:54Z

vnext/src/Libraries/Components/AccessibilityInfo/AccessibilityInfo.windesktop.js

    return new Promise((resolve, reject) => {
-      // TODO Hx: Implement this module.
-      return false;


false [](start = 13, length = 5)

This is so hilariously wrong, even for a stub...

NickGerleman · 2020-04-14T13:33:07Z

vnext/src/Libraries/Components/DatePicker/DatePicker.tsx

@@ -16,7 +16,7 @@ const styles = StyleSheet.create({
  },
 });

-const RCTDatePicker = requireNativeComponent('RCTDatePicker');
+const RCTDatePicker = requireNativeComponent<any>('RCTDatePicker');


any [](start = 45, length = 3)

This is a tad lazy, but the native component interface doesn't match the JS component, is nontrivial, and we're we're going to jettison this and reimplement it anyway.

NickGerleman · 2020-04-14T13:35:05Z

vnext/src/Libraries/Components/Switch/SwitchNativeComponent.windows.js


 type SwitchChangeEvent = $ReadOnly<{|
  value: boolean,
 |}>;

-type SwitchProps = $ReadOnly<{|
+type NativeProps = $ReadOnly<{|


NativeProps [](start = 5, length = 11)

These are of course different from before :)

We need to update our ViewManager still

NickGerleman · 2020-04-14T13:36:44Z

// const SnapshotViewIOS = require('./examples/Snapshot/SnapshotViewIOS.ios'); [Windows]

Changed our TS to be the IOS version with a couple things commented out to make future merges easier. Added an item to followupon sharing code here.

Refers to: vnext/src/RNTester/js/RNTesterApp.windows.js:19 in cf171aa. [](commit_id = cf171aa, deletion_comment = False)

NickGerleman · 2020-04-14T13:38:45Z

import {Flyout} from '../../../../Libraries/Components/Flyout/Flyout';

This here and in other places was to address a build order issue exposed when we changed where out main lived.

Refers to: vnext/src/RNTester/js/examples-win/Flyout/FlyoutExample.windows.tsx:9 in cf171aa. [](commit_id = cf171aa, deletion_comment = False)

NickGerleman · 2020-04-14T13:39:24Z

// This is a port of TextInputExample.ios.js

Need to follow up on recreating this

Refers to: vnext/src/RNTester/js/examples-win/TextInput/TextInputExample.windows.tsx:7 in cf171aa. [](commit_id = cf171aa, deletion_comment = True)

NickGerleman · 2020-04-14T13:57:16Z

vnext/src/Libraries/Components/StatusBar/NativeStatusBarManagerAndroid.windows.js

+    +HEIGHT: number,
+    +DEFAULT_BACKGROUND_COLOR: number,
+  |};
+  // [Windows Allow NativeOrDynamicColorType to make Flow happy (this will never be called)


This is totally not the right way to fix this, but is the least invasive until we remove NativeOrDynamicColor entirely in the next release, and can't ever run anway. I.e. it's temporary.

…r for baseFileNotFound

…ve-windows into 62-ready

* Upgrade to 0.62 * Update the issue number in a few override entries * Image.ios -> Image.windows * Update override-tools UT to Expect Override File as a validation error for baseFileNotFound Co-authored-by: Andrew Coates (REDMOND) <acoates@microsoft.com>

Upgrade to 0.62

cf171aa

NickGerleman commented Apr 14, 2020

View reviewed changes

Update the issue number in a few override entries

a41d7d1

NickGerleman commented Apr 14, 2020

View reviewed changes

Image.ios -> Image.windows

9526717

NickGerleman changed the title ~~[Draft] Upgrade to 0.62~~ Upgrade to 0.62 Apr 14, 2020

NickGerleman marked this pull request as ready for review April 14, 2020 14:04

NickGerleman requested a review from a team as a code owner April 14, 2020 14:04

NickGerleman added 2 commits April 14, 2020 07:11

Update override-tools UT to Expect Override File as a validation erro…

b956a13

…r for baseFileNotFound

Merge branch '62-ready' of https://github.com/NickGerleman/react-nati…

bb3ed48

…ve-windows into 62-ready

This was linked to issues Apr 14, 2020

Upgrade to 0.62 #4374

Closed

Update React Native for Windows to target RN 0.62 #4025

Closed

Remove forked Switch.tsx/js file from RNW #3188

Closed

acoates-ms approved these changes Apr 14, 2020

View reviewed changes

NickGerleman merged commit ae37f8e into microsoft:master Apr 14, 2020

		@@ -0,0 +1,96 @@
		/**
		* Copyright (c) Facebook, Inc. and its affiliates.


		var TOUCH_EXPLORATION_EVENT = 'touchExplorationDidChange';
		import NativeAccessibilityInfo from './NativeAccessibilityInfo';

Upgrade to 0.62 #4587

Upgrade to 0.62 #4587

Conversation

NickGerleman commented Apr 14, 2020 • edited Loading

Microsoft Reviewers: Open in CodeFlow

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickGerleman Apr 14, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickGerleman commented Apr 14, 2020

NickGerleman commented Apr 14, 2020

NickGerleman commented Apr 14, 2020

Choose a reason for hiding this comment

NickGerleman commented Apr 14, 2020 •

edited

Loading

NickGerleman Apr 14, 2020 •

edited

Loading