0.57.3 Discussion

[grabbou]

Conversation on this thread are limited to 0.57.3 release's major issues and backport (cherry-pick) requests from commits that are already on master - so that we can then produce a 0.57.4 release in the near future.

An example of a good such request is a bug fix for a serious issue that has been merged into master but did not make the 0.57.3 cut.

In other words, if you cannot point to a particular commit on master, then your request likely belongs as a new issue in http://github.com/facebook/react-native/issues.

[ahce]

@grabbou when release 0.57.3 in npm?

[kelset]

Hey everyone, there was an issue with the release, I'm looking into it today

Ok, release is out now on npm.

One thing: when you upgrade to this version you NEED to upgrade react and react-test-renderer to version "16.6.0-alpha.8af6728". It's safe, I've tested 0.57.3 in multiple apps without issues. (if you prefer to not, you can wait for future 0.57.x releases, 16.6.0 will be most likely released in a couple weeks at ReactConf).

[grabbou]

We should cherry-pick facebook/react-native@b44c5ae to improve Xcode 10 support and make sure it's all good at the time of 0.58 release.

[peat-psuwit]

Please cherry-pick Update link command for Android project. This commit fixes Android project auto-linking to be compatible with already-released Gradle plugin upgrade.

[matei-radu]

@peat-psuwit that update did not cover unlinking, so I think it should only be cherry-picked together with the unlink PR when it lands.

[peat-psuwit]

@matt-block I agree with you. Thank you for pointing it out!

[radeno]

@grabbou maybe we should have one up-to-date comment with approved cherry-pick commits. Like in forums where first topic comment is always updated (eg instructions to install something for current app version). Something like @kelset do in previous version #46 (comment) What do you think? It allows us to update changelog continuously.

Updated With @kelset help

Anyway some patch commits:

• Avoid calling lazilyLoadView when running in the debugger facebook/react-native@4f75709
• Fix Xcode 10 errors relating to third-party facebook/react-native@b44c5ae
• OkHTTP update facebook/react-native@10fc548
• When paddingStart is 0, it should override paddingHorizontal facebook/react-native@f90c9b5
• Cleaning up imports in ViewGroupManager facebook/react-native@082a869
• React sync for revisions d836010...4773fdf facebook/react-native@8325e09
• Android: Fix DynamicFromMap object pool synchronization facebook/react-native@b0d68c0

[kelset]

@radeno:

• the NDK one is already in 0.57.3
• You link a couple of Fabric related commits, I don't understand why we should cherry-pick those - is there any particular reason? 🤔
• Unless there is a strong need for new Flow I don't think we should bump it in a patch-level release

[radeno]

@kelset
You are right about all points. My post is updated. So slimmer.

[yinhangfeng]

0.57.3 android have a accessibility role crash issue facebook/react-native#21754
it happens because incomplete cherry-pick from master facebook/react-native#21754 (comment)

[kelset]

Hey @yinhangfeng thanks for pointing that out - if I understand correctly, we need to cherry pick these two

• facebook/react-native@1f96ff6
• facebook/react-native@139559f

to fix it right?

( cc @peat-psuwit)

[peat-psuwit]

The actual fix is facebook/react-native@1f96ff6, but do cherry-pick facebook/react-native@139559f first to avoid conflict. (Also, revert facebook/react-native@1592a8d)

[kelset]

@janhesters as mentioned way too many times across these issues, please keep the conversation on commits already on master.

That said, on testing, there are things you could do via PRs, like this to help with it. And also you could submit PRs to the changelog: there is a "known issue" section precisely for those kind of scenarios.

[janhesters]

@kelset I'm sorry, my mistake.

I think I'm not good enough create a PR and fix this issue (I wouldn't even know where to start). But I did a PR for the changelog. Thank you! 😊

[radeno]

Updated.
Another posible fixes to cherry-pick:

• RN, Android: Fix pull to refresh refresh component clipping on Android facebook/react-native@8a3a0ad
• RN: ListView requestAnimationFrame leak facebook/react-native@70b5eb3
• Android: Back out "[react-native][PR] Rounded corner rendering fixed on Android N." facebook/react-native@bb407fa
• iOS: Fix issue when inserting text at 0 when maxLength is set facebook/react-native@36507e4
• RN, iOS: Bump [email protected] facebook/react-native@b951499
• Android: Fix onTextLayout metrics on Android when using alignText facebook/react-native@1c240ae

[mikemorris]

• Android textTransform style support facebook/react-native@22cf5dc

[janicduplessis]

facebook/react-native@b0d68c0 fixes a crash that happens with react-native-svg (and possibly others) on Android.

[kelset]

Thanks everyone, I'll try to review & finalise the list of cherry-picks today & trigger the release this weekend (has been sick for the past two days 🤢).

[Titozzz]

Heya, facebook/react-native@fd744dd might be interesting !

I know that some folks at meliorence/react-native-snap-carousel#203 would enjoy it 😄

[eldh]

Would love to see facebook/react-native@fa6035b get into next release. It should fix testing with Appium (see facebook/react-native#21238)

[kelset]

@eldh that commit is already in 0.57.3 -> https://github.com/react-native-community/react-native-releases/blob/master/CHANGELOG.md#android-specific-fixes

[eldh]

Awesome, don't mind me then 😄

[kelset]

Hey everyone - based on the list of commits listed above (plus a few more) I attempted a cherry-pick test locally to prep 0.57.4. It wasn't either easy or funny, so I didn't end up pushing to the remote branch what I've done.

Here's the breakdown, so that either myself or @hramos or @grabbou can pick it up quickly again - and we can discuss if we want to proceed with a subset of these commits or something.

I've managed to cherry-pick and test with everything ✅ these commits (ordered in the same order I cherry picked them):

• the fix described here which actually triggers a conflict on AccessibilityDelegateUtil but copy/pasting the version from master is basically a valid way to correct the conflict so I went with that
• Do not use fbjs/emptyObject
• Fix crashes on invalid regex
• Consolidate native dependencies versions
• bump okhttp3 to 3.11
• Fix DynamicFromMap object pool synchronization
• Fix pull to refresh refresh component clipping on Android
• Remove TimerMixin from ListView
• ListView requestAnimationFrame leak
• Back out "[react-native][PR] Rounded corner rendering fixed on Android N."
• Fix issue when inserting text at 0 when maxLength is set
• Bump [email protected]
• Text: send metrics after rendering (iOS)
• Android: Send metrics in onTextLayout events
• Use TextLegend example in Android as well
• Fix onTextLayout metrics on Android when using alignText
• Allow specifying iOS version for run-ios with simulator option
• Relax the requirement that lazy module cannot be initialized on the main thread
• Add deprecation notice to SwipeableListView
• Cleaning up imports in ViewGroupManager
• Android textTransform style support

Aside from those (and here starts the complex stuff) the React sync for revisions d836010...4773fdf adds a warning about context, more precisely

Deprecate context object as a consumer and add a warning message

And I'm pretty confident we don't want to spam it to everyone.

THEN, I also wanted to cherry pick those:

• Fix rounding of negative numbers
• When paddingStart is 0, it should override paddingHorizontal

But they forced me to get into the spiral of these commits

• Remove Node 10 check
• Add missing copyright headers
• Update copyright headers to yearless format

at least, and then everything was a mess of conflicts so I gave up of getting them in.

Lastly, other 3 I wanted to cherry pick but had conflicts not simple to fix

• use requireActual and requireMock from jest instead of require
• Avoid calling lazilyLoadView when running in the debugger
• Fix Xcode 10 errors relating to third-party

I know the last one should help with Xcode 10 but it's a mess to cherry pick 😞

---

So, after all this, I believe that we should proceed with a 0.57.4 with the first list I presented.
LMK

[matthargett]

It looks like the noisiness of that React Context warning was lessened in a later commit:
facebook/react@9ea4bc6

But if we don't have a way forward with React 16.6 stable, then I would propose reverting the React sync cherry picks from before and staying on 16.5 for the rest of the 0.57 releases. Taking an LTS onto a React alpha make me queasy, but leaving it on a React alpha is not great and gives me flashbacks to React Native 0.43.

[fungilation]

Thanks for all the cherry picks @kelset! Hope you are recovering.

On React alpha, I for one don't want to upgrade to RN 0.57.3 because of it. As a matter of principle, RN having no alpha or beta labeling shouldn't ever depend on alpha/beta versions of critical components. Why the rush? Let React 16.6 stabilize before cherry picking all the sync commits associated.

Looking at https://github.com/facebook/react/blob/master/CHANGELOG.md, I don't see anything important between the minor versions (16.5.0 -> 16.5.2). I'm perfectly fine on React 16.5.0.

[a-tarasyuk]

@kelset Thanks for the detailed update, and for your effort to prepare this release. I hope that Fix Xcode 10 errors relating to third-party will be merged to 0.57.4 ;) - it contains many changes, however, it can resolve annoying issues with the new Xcode.

[kelset]

Thanks for everyone's feedback.

Related to the React alpha issue: I understand, but we are talking about an alpha of a minor version update, which is less likely to create issues (I haven't found any during my time with 0.57.3 in our prod app nor during my testing).

Since it's the weekend and my test yesterday wasn't quick nor easy, here's my proposal for now: this coming week there's ReactConf (October 25 & 26). So we'll soon have a clearer idea of when 16.6.0 will be out: I expect (but have no knowledge) that it will be out during the event, but if that's not the case (ex. they decide to release it in a month) we'll reverse the React sync.

This will mean delay the release of 0.57.4 of (at least) another week.

Or, we do a 0.57.4 with the "already tested" subset of commits I listed above and then for 0.57.5 we consider the 16.5.0 rollback (which means that people can't use React 16.5.2 btw).

LMK

[fungilation]

Since what's done is done and I've said my piece regarding syncing with alpha, I'm for waiting another week if React 16.6.0 may likely be out then. Least work option, unless it's not out then.

[a-tarasyuk]

In my opinion better to release RN which will work correctly with stable React version. (16.5.2 or 16.6.0). Waiting one week to have a stable RN version which will work with stable React (does not matter 16.5.* or 16.6.*) version is absolutely OK.

[fungilation]

You/I have RN 0.57.2 on React 16.5.0.

[ikesyo]

I want the following to be cherry-picked

• Re-introduce UIImplementationProvider facebook/react-native@b002df9

[ahce]

Is possible RN 0.57.4 with React 16.5.0?

[grabbou]

Personally, I don't mind using an alpha version for a while as React team is wrapping up the stable release. However, I am getting constant impression that the community is having a different opinion in that matter and prefers staying on a stable bandwagon, even if that means missing out on small optimisation and improvements for a while.

That said, I think we shouldn't publish a stable release of React Native that depends on an alpha version of React (even if that's an unstable release of a minor bump - there's always policies at companies). Let's make it a rule from now on.

Now, since we have already released React Native 0.57.3 with an alpha dependency, I would say we continue our cherry-picking process the regular way. That said, I second what @kelset already said. Let's push 0.57.4 out for those who might need these changes and wait for a week to decide whether to rollback or sync with latest.

PS. If someone wants an alpha version of React with a stable React Native release, we can always release 0.57.3-alpha to match React dependency for the time being.

[gwmccull]

@grabbou I'm in 100% agreement about not using an alpha of React. I had to do the upgrade to the version of React Native that used React 16 alpha (might have been 44?). It was terrible. Enzyme didn't support the alpha version of React and a lot of other libraries were broken

I don't think any new feature of React is worth that pain

[kelset]

Thanks everyone for contributing to the discussion - we'll keep it in mind for future releases (we are also planning a core meeting precisely on these subjects in early November).

That said, I've spoken with the core team and the FB internal team and since "A non alpha version of React will be landing soon" I won't revert the commit (but won't cherry pick the new one I mentioned earlier) - and proceed to do again a test 0.57.4 local & eventual release.

If you prefer to not use 16.6.alpha you will have to wait a big longer - but, again, in terms of "real usage" I haven't had any problem so far with it (our main app in prod has 0.57.3, been out since last weekend, no issues).

[mikemorris]

• Android Gradle plugin 3.2 directory path fix facebook/react-native@ff084a4

[ikesyo]

@mikemorris That is already in 0.57.2 facebook/react-native@a4fed6e.

[fungilation]

Well, I see React 16.6.0 already out.

[kelset]

couple of updates:

• Yes, as expected, 16.6.0 has been released. But the React sync commit to provide "first class" support to it in react-native is not yet out, and it may take some time (I have no further info on this)
• because of the GitHub outage, basically CircleCI played funny so no CI jobs were run until, like, late yesterday night
• the CI on 0.57-stable is currently red for the Android test, there is a PR that should fix it (Make google repo priority higher than jcenter facebook/react-native#21910) - since it's a minor change I may attempt it locally in the branch but I'm really busy today so not sure

[dvicory]

An extra vote towards cherry-picking facebook/react-native@b002df9. We need this for react-native-navigation.

[ikesyo]

• Quote "$NODE_BINARY" in react-native-xcode.sh facebook/react-native@7d4e94e

[matt-oakes]

It would be good to get this merged PR cherry-picked as the issue is causing crashes on Android phone:

PR: facebook/react-native#20913
Commit: facebook/react-native@09c78fe

[kelset]

hey everyone, thanks for all the feedback and participation. CircleCI was stuck again (and I was really busy) so I preferred to move forward with the set of commits I have already tested.

0.57.4 is out (I've tested it "after release" and didn't have any issues), the new discussion is here -> #54

(I've already added the two newest commits requested there)