Unify all the macOS diff tags

[Saad Najmi (Saadnajmi)]

Please select one of the following

• I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
• I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
• I am making a fix / change for the macOS implementation of react-native
• I am making a change required for Microsoft usage of react-native

Summary

This PR resolves#1579 by doing a few things:

1. Aligns all of the various diff tags across the repo. The next person who tried to mass-change the diff tags should have a much easier time now that they all match. Discussion for the tag format happened in Upstream RCTSRWebsocket.m bug fix #1597 and I'm going with a modified version of Option C (I'm using // macOS] instead of // ]macOS for end tags)
2. Adds a markdown doc page as docs/DiffsWithUpsteam.md to document the format of the diff tags. This should be helpful to new and external contributors to the repo.
3. Refactors some code to match the above documentation. Specifically, I:
   • Swapped some ifdef blocks around so that the original iOS code always came first
   • Made sure that the first #if !TARGET_OS_OSX had a single line tag, while the other #else/#endif had block tags

I did my best to keep this a comment / format only change. To that effect, I did quite a bit of manual work and triple checking to make sure things are consistent. Because I'm effectively going through every diff in our repo, I did notice many places we can make simple one line improvements / usptream changes / diff removals. I attempted to document as many as I could in the following GitHub project: Upstreamable changes / removable diffs . However, I'm sure there were a few small changes that have snuck into this change as well (removing unnecessary diff tags, unused imports or variables, etc).

Changelog

[macOS] [Fixed] - Unify all the macOS diff tags

Test Plan

There was a lot of manual going through all the tags. And then a second time. And then a third time 🙃. I'm going to trust the CI to make sure I didn't break anything.

[Harrie Shin (harrieshin)]

i don't know how i feel about this. bracket means pretty different thing in objc so as i read the functions i would be confused.

[Saad Najmi (Saadnajmi)]

i don't know how i feel about this. bracket means pretty different thing in objc so as i read the functions i would be confused.

Good point, that might be why we went with the weirder format of " // ]tag" instead of "// tag]". Open to other ideas. I don't expect to merge this for a while...

[Adam Gleitman (amgleitman)]

One issue I have is that it's a lot easier to search for TODO(macOS GH#774) in our code when we're trying to find specific differences between RN-macOS and RN Core. Simply searching for macOS could give us a lot of false positives.

[Code Analyser (analysis-bot)]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: 21dbc67
Branch: main

[Saad Najmi (Saadnajmi)]

Update on this PR: I'm currently doing a "final" pass through all the files, but obviously that takes time.. I'm at 159/552 . Once I've gone through them all, I'll add a documentation page and mark as ready for review.

[Saad Najmi (Saadnajmi)]

Hello, I am dividing this PR into 3 chunks and assigning one to each of the people tagged. Please read the PR Description and the documentation added in 63648a117972414777b3f1aadbb4923189087a92 to get an idea of what has changed and what to look for! 90% of the changes are mechanical, but there is a 10% that includes changes like:

• Adding a Github Issue for a diff block
• Swapping the iOS/macOS parts of a #if !TARGET_OS_OSX block to match the rest of the repo
• Typo fixes

So please keep your eye out for changes like that!

The blocks are as follows:
Chunk 1: up to Libraries/WebSocket/RCTSRWebSocket.m - Lynn Zhang (@lyzhan7)
Chunk 2: Libraries/WebSocket/WebSocket.js - React/Modules/RCTUIManager.m
- Mike Schreiber (@mischreiber)
Chunk 3: React/Modules/RCTUIManagerObserverCoordinator.h + - chiuam

Thank you again!

[Mike Schreiber (mischreiber)]

Signed off for my set of changes, with a couple of minor (non-blocking) comments!

[Lynn Zhang (lyzhan7)]

Overall looks good! Left some comments on some tags that might need their brackets adjusted. There are also a lot of other comments about stuff that I realized are not related to your PR, such as whether a tag should be added or not - feel free to won't fix since your PR is already pretty big (I saw the project board you opened, could be added to that?). I was just commenting on anything I noticed that didn't make sense to me.

The docs/DiffsWithUpsteam.md was great - some other questions that came up while I was reviewing:

• (I think Mike already asked this) can there be nested macOS diff tags? Personally I find them confusing - in what scenarios should they be used?
• When we use block tags vs. single line tags is a little inconsistent. I saw a couple places where we have // [macOS] at the end of an if statement. I would assume that the entire if block was added for macOS. In other places it's a bit more clear, where we have two tags wrapping the whole if statement
• I think you mentioned this at some point while discussing how to format tags - should tags always be on the same line as some other code, or always on its own line? Just noticed some inconsistencies with this
• I found some of the compound if statements/ifdefs that had a macOS tag at the end a little ambiguous. Was the entire if statement added for macOS, or was the if statement already there but we just modified the condition for macOS? I commented on one of the places where I saw this (#if !TARGET_OS_UIKITFORMAC && !TARGET_OS_OSX)

If you have answers to the questions (maybe the answer is just "both options are fine"), might be good to add to the md file.