feat: improve upgrade to use rn-diff-purge

[thymikee]

Summary:

Upgrading experience is a bit harsh for React Native. We either need to install global react-native-git-upgrade and try that, or patch projects manually using tools like rn-diff-purge.

Since rn-diff-purge gets some recognition in RN core and website and it has an automated path of applying patches, I thought that it would be nice to use it in our upgrade command.

What the command does now:

1. Determine version to upgrade and run some validation checks
2. Fetch patch between current version and specified from rn-diff-purge and rename diff app to current project name
   1. If the patch is empty, install RN and its peer deps
   2. Exit with success
3. Prepare for 3-way merge by creating temporary patch file and adding new git remote
4. Apply patch with a fallback to 3-way merge, exclude package.json because it will always be failing
5. Install RN and its peer deps
6. Cleanup temporary files and git remote
7. Exit with success

This PR is technically breaking change, so I'm cool with introducing a temporary new command or put old one behind a different name.

Test Plan:

Added tests with various scenarios.

Successfully upgrading to specified version:

Failed to fetch a non-existent diff:

No upgrade needed:

Dependency mismatch:

Upgrading to older version:

[elicwhite]

This looks awesome and I’m excited to make this experience better for people. I’ll defer to the typical CLI folk as well as @pvinis

[cpojer]

This is super awesome, thank you so so much for doing this, it's exactly what I hoped we could do, and you already did it!

Can you just go ahead and make this the default? Let's dump the old way of doing things, it is clearly very problematic for people – what you are doing here cannot be any worse. All we need to make sure is that the right diff is available when we make a new release. As part of that, we may want to consider merging rn-diff-purge into react-native-community. What do you think?

[pvinis]

I am currently doing some testing on this, to make sure a couple cases are handled well, but it looks good so far!

[thymikee]

Yep, rn-diff-purge should be also fully automated so that once the npm tag is released, a diff comes up few minutes later. Looks like it's tracked in pvinis/rn-diff-purge#10. @pvinis would you mind moving the repo to react-native-community?

Can you just go ahead and make this the default? Let's dump the old way of doing things, it is clearly very problematic for people

Well technically 0.59 (our first consumer) is still in RC, so we could stretch that semver incompatibility, and put the old version behind a flag.

Looks like neither Expo nor Ignite use the upgrade, so we should be fairly safe

[cpojer]

Yep definitely recommend swapping the default to this, and keeping the old one around for a little longer. Now is a good time.

[pvinis]

I am happy to move this to community, sure, I don't see why not.

For automatic generation, right now I was trying to make a rpi build the diff when an npm version is up, but the npm version is not the easiest. I found a couple of services that email me so I was thinking of hooking this up with a mail check, but I saw npm has hooks but only for a paid org. Any ideas are welcome.

I agree that we might as well make it the default since the previous one seems to be not much used, but it would make sense to keep both for a while, one default and one with a flag.

Also, here are the things I was concerned about:

• What happens in case of conflicts.
  I saw that the upgrade stops with the files as they are, so the user fixes the conflicts, but the package.json is left untouched. I think we should also update that, so when the conflicts are fixed, the upgrade can be complete.
• What happens with linking libraries in XCode (like JSC recently) when we have more targets.
  For this, it might be a problem until we move to something like xcodegen? Maybe still this doesn't solve it?
  After testing it, it's as I expected, the change happens only on the first target, and not any more, which makes sense but sucks. Since there is no better way for upgrading the xcodeproj files, I guess it's fine to just keep it like that, and hopefully find a better way soon. Maybe we could show a text that the same changes should happen manually by the user in other targets? (I need to find some time to make a PR with xcodegen and xcconfigs, or even better, instead of diff we use a script for xcode that can loop over targets and do our changes.)
• What's the git status after the upgrade, which files are staged, unstaged, and why?
  I saw that all files are staged except package.json and yarn.lock. I would say let's stage all or leave all unstaged.
• Tags
  I saw that if a tag version/x.y.z exists, it's not replaced. If I try to create a tag after an upgrade like version/x.y.z, it's taken by the cli upgrade commits from purge, so that's kinda weird, because then as a user I need to go delete a tag. At least I didn't see problems from having tags with the same name while upgrading.

[pvinis]

It would be very useful to have a link to the diff we are doing when it fails.

A link to pvinis/rn-diff-purge@version/0.44.0...version/0.47.2 would be useful to see the diff and our conflicts and see what's up.

[pvinis]

Actually, after a conflict, the upgrade stops in whatever point it was. I think it might be better to keep it going, so we can see the rest, and solve the conflict.

[pvinis]

I tried using it in an actual project and in a test project with a few "bigger" changes like some babel configs, some different xcode setup, typescript etc.
It didn't go well. I was not expecting it to go well, since git patches of a template are not enough to upgrade an actual project, but I wanted to try.

I think even with this command, I will still use the manual way to upgrade. We should think about having a migration script instead, that does some work per version. This would need to be manually created.

Let's say there is a new version. We use purge to make the diff. We see the diff, and based on that we have a migration script do the change. If the change is just an edit of a file, like package.json update or babel update etc, we apply it. For android it's mostly simple. For Xcode we can use some library to edit the xcodeproj and do the change (for example linking JSC) but we see that in the tempate (and so in purge) we link the lib to the target. In the migration script we would loop over the targets and link the lib. Then most other changes are small edits in the AppDelegate or App.js etc, and for these again, if we can apply them, good, if not, the script can notify the user, so they do the change.

Maybe what I'm thinking is basically a manual upgrade helper, that applies what it can using a script instead of diff, then applies usin the diff, then guides the user to do the rest of the changes.

[cpojer]

@pvinis that's a great idea. I think it would be great if the upgrade command could also print a condensed version of the release notes, like the breaking changes and links with guidance on what to do.

[elicwhite]

Another option is to bail out really early from applying things automatically and to just show the patches. Essentially just an interactive cli walkthrough of the changes in Rn-diff-purge. We can automate that more and more over time when we are confident.

[turnrye]

@cpojer we can definitely do that; with libs like changelog-parser it just works™.

[thymikee]

@pvinis thanks for great feedback. This command isn't meant to be a perfect way to upgrade, but it should ease the process as much as possible.

A link to pvinis/rn-diff-purge@version/0.44.0...version/0.47.2 would be useful to see the diff and our conflicts and see what's up.

Added.

What happens in case of conflicts.

We now show the git status output.

What happens with linking libraries in XCode (like JSC recently) when we have more targets.

We fail miserably, but we're able to show the diff and point to the plain diff, so it's still way better than any current workflow.

What's the git status after the upgrade, which files are staged, unstaged, and why?
I saw that all files are staged except package.json and yarn.lock. I would say let's stage all or leave all unstaged.

We now show git status and add package.json and common lockfiles to git stage.

Tags
I saw that if a tag version/x.y.z exists, it's not replaced. If I try to create a tag after an upgrade like version/x.y.z, it's taken by the cli upgrade commits from purge, so that's kinda weird, because then as a user I need to go delete a tag. At least I didn't see problems from having tags with the same name while upgrading.

I'm not sure what you mean, could you clarify?

[pvinis]

Thanks for addressing the above. I'm looking forward to an Xcode solution in the future :D.

Let me rephrase the tag thing.

Currently, this new upgrade command will basically bring the purge commits and tags in the user's project. If that user has tags in that project, that have the same form as the tags in the purge repo, then the tag will not be in the purge commit, but it will stay in the user's project commit.

On the other hand, if we bring all the commits and tags from purge to the user's project, if that user wants to create a tag with the same form, like if they make a new app release and the tag is version/0.38.0, that tag will be already taken. The user will have to go find the tag and delete it in their repo.

Do you know what I mean, or did I not explain it very well? My concern is basically that we will be messing with tags in a repo that's not ours. But since we remove the remote after, maybe it's not really needed to do anything more. It was just a thought.

[thymikee]

Oh no, we must do something about it

[thymikee]

@pvinis found that since git 2.13 (Q2 2017) there's git fetch --no-tags that does the trick.

[thymikee]

Latest iteration on the UI:

[pvinis]

Awesome. I'll run it again. 👍

[thymikee]

@pvinis anything that's holding us back with merging this? I'll be unavailable for the next couple of days so I'd like to get this over the finishing line.

I also have some ideas for further improvements:

1. fallback to generating the diff locally if diff is not available (the actual procedure is quite straightforward, the only shortcoming is the time to generate it, which involves installing RN twice from scratch)
2. With 1) implemented, we could easily support custom templates, which would produce way better diffs than from the default one
3. Transfer the rn-diff-purge to react-native-community (tracked in Feature request: move this project to react-native-community pvinis/rn-diff-purge#18)

[pvinis]

I think we can merge. I like the ideas too :). sorry for the delay.

[thymikee]

@Esemesek @grabbou looking for a final look from you 👀

[cpojer]

Amazing, I'm so excited about this. Thanks @thymikee and @pvinis. Can you also make sure to improve the upgrading guide on the website? :)

[thymikee]

@pvinis mind taking care of the docs?

[pvinis]

You took care of the docs before I saw this comment, and you did a great job too! :)

[darknblack]

Thank you and great work =)

[vomchik]

I love it!