Tracker to follow up with react-native-web patches

[Beamanator]

Purpose of this issue is to regularly check on the upstream PR so we can ideally get rid of the patch-package fix we had to implement in App

Patches:

1. patches/react-native-web+0.19.9+001+initial.patch & patches/react-native-web+0.19.9+002+fix-mvcp.patch
   • App PR: Upgrade to react-native-web v0.19.9 #24482
   • Upstream PR: ?
2. patches/react-native-web+0.19.9+003+measureInWindow.patch
   • App PR: [CP Staging] Fix/issue 31069 #31184
   • Upstream PR: fix: use getBondingClientRect instead of getRect for measureInWindow necolas/react-native-web#2613
3. patches/react-native-web+0.19.9+004+fix-pointer-events.patch
   • App PR: fix: 31101 Eye icon to show password on password protected PDF file is not working #31176
   • Upstream PR: ?
   • Upstream issue: Pointer event is not applied for grand-children necolas/react-native-web#2606
4. patches/react-native-web+0.19.9+005+image-header-support.patch
   • App PR: Image Web/Desktop: Add support for http headers #13036
   • Upstream PR: Image: support ImageSource with headers necolas/react-native-web#2442

We don't currently have an Expensify fork of react-native-web, we recently updated to v0.19.9 and we have the above patches in the patches folder.

cc @roryabraham , do you think it's worth tracking these patches & creating upstream PRs for all of them, so we can try to remove the patches eventually?

[roryabraham]

@Beamanator Going to help fill this in as best as I can. At some point I also created a spreadsheet for a similar purpose

[roryabraham]

do you think it's worth tracking these patches & creating upstream PRs for all of them, so we can try to remove the patches eventually?

In short, yes I think it's worth it

[roryabraham]

Just going to list all the open upstream PRs in react-native-web that I'm aware of originating from people acting on Expensify's behalf:

• fix: use getBondingClientRect instead of getRect for measureInWindow necolas/react-native-web#2613
• Fix: VirtualizedList passive event listener warning necolas/react-native-web#2601
• Implement maintainVisibleContentPosition necolas/react-native-web#2588
• Fix: Multiline Text though clipped can be scrolled into view necolas/react-native-web#2584
• fix: measureLayout relative to ScrollView returns incorrect y value necolas/react-native-web#2570
• Remove collapsable props from Animated component necolas/react-native-web#2569
• Fix Image Caching necolas/react-native-web#2493
• Image: support ImageSource with headers necolas/react-native-web#2442

[Beamanator]

@roryabraham oh nifty! I like the spreadsheet idea too - we coulddd just link to that spreadsheet from this issue, if that is better? 🤷

[Beamanator]

And thanks for adding to the list... I wonder if we should just keep adding patches or if we know the RNW maintainer is planning to review those PRs eventually?

#27174 for example - we've been "Holding" on the Upstream PR for more than 2 months at this point

[roryabraham]

Well if there's a specific PR and we're unsure of the status we can try to contact Meta and/or Necolas using discord. I want to be respectful of their time though, so I'm happy to reach out as long as:

• there's an issue report with a minimal reproduction
• we have an open PR to resolve the issue
• they have made no recent comments on the issue or PR and have not previously made any comments to indicate when or why the PR may or may not be reviewed

So basically we can take it case-by-case but for a major dependency like react-native-web we should only rely on patches if we have a reasonable expectation that the patch or alternate solution will be merged upstream.

What we'd want to avoid is a scenario where we want a feature so we build it in a patch but don't try to get it merged upstream, or worse - the maintainer rejects an upstream solution and then we proceed with a patch anyways

[Beamanator]

I like that as being some sort of standard process! So for the issue I mentioned above...

• Issue report with reproduction steps: mWeb - Multiline Text's clipped part is scrolled into view by dragging selection cursor necolas/react-native-web#2582
• Open Upstream PR: Fix: Multiline Text though clipped can be scrolled into view necolas/react-native-web#2584
• No comments on issue or PR ✅

In this case, the patch would be super tiny, so I don't think it's a waste of time to at least get the fix in to our codebase, then keep waiting on maintainer to respond. What you think?

[roryabraham]

Sure, as long as the PR that introduces the patch in E/App links to the upstream issue and PR

[Beamanator]

Makes sense to me. I don't have a lot of time for this at the moment, but I still think the priority of all this is Monthly - so I'll get around to this eventually. What I'm thinking is:

1. Recommend this (with slight improvements) as a formalized plan in Slack
2. If we get agreement, write an SO
3. Maybe make a GH issue template for this kind of tracker 🤷 (maybe not)
4. Use new process to evaluate which PRs in this tracker to create as patches / which not to

[Beamanator]

Not priority yet

[Beamanator]

Same

[Beamanator]

same

[Beamanator]

same

[Beamanator]

same

[Beamanator]

same

[roryabraham]

I somewhat recently added some simple enforcement that will help patches from being ignored during package upgrades: #45098

Given that this isn't likely to become more of a priority soon, and in all the current cases there isn't much we can do to speed things along, I'm going to close this.