Skip to content

Commit 0c58be9

Browse files
authored
Merge pull request Expensify#63674 from software-mansion-labs/war-in/improve-patch-guidelines
[No QA] Improve patch guidelines
2 parents d41364f + bfa2c5e commit 0c58be9

7 files changed

Lines changed: 113 additions & 19 deletions

File tree

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
name: Patch Tracking
3+
about: Template for tracking patches applied to third-party libraries
4+
---
5+
6+
If you haven't already, check out our [contributing guidelines](https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md) and [patch guidelines](https://github.com/Expensify/App/blob/main/contributingGuides/PATCHES.md) for information on how to manage patches.
7+
___
8+
9+
## Patch Information
10+
11+
**Library Name:** <!-- e.g., react-native-pdf -->
12+
**Library Version:** <!-- e.g., 6.7.3 -->
13+
**Patch Number:** <!-- e.g., 001 -->
14+
**Patch Description:** <!-- e.g., update-podspec-to-support-new-arch -->
15+
**Full Patch Filename:** <!-- e.g., react-native-pdf+6.7.3+001+update-podspec-to-support-new-arch.patch -->
16+
17+
## Patch Details
18+
19+
### Reason for Patch
20+
<!-- Explain why this patch is necessary. What issue does it solve? -->
21+
22+
### Changes Made
23+
<!-- Briefly describe the changes made in the patch -->
24+
25+
### Upstream Status
26+
**Upstream PR/Issue:** <!-- Link to the PR or issue in the upstream repository, if one exists -->
27+
28+
## Related Information
29+
30+
**PR Introducing Patch:** <!-- Link to the PR that introduced this patch -->
31+
32+
## Additional Notes
33+
<!-- Any other relevant information about this patch -->
34+
35+
## Checklist
36+
- [ ] Patch file is correctly named and placed in the appropriate directory
37+
- [ ] Patch is documented in the corresponding `details.md` file
38+
- [ ] This issue is linked in the `details.md` file
39+
- [ ] Upstream PR/issue has been created (if applicable)

contributingGuides/PATCHES.md

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,34 @@ To ensure patches remain maintainable and traceable over time, follow the guidel
66

77
---
88

9+
## 🛠️ Creating a Patch
10+
11+
To create a patch for a third-party library that you've modified:
12+
13+
1. Make your changes to the library code in the `node_modules/<edited-library>` directory
14+
2. Run the following command to generate the patch file:
15+
16+
```
17+
npx patch-package <edited-library> --append "<short-patch-description>" --patch-dir ./patches/<edited-library>
18+
```
19+
20+
For example:
21+
```
22+
npx patch-package react-native-pdf --append "fix-pdf-rendering-on-ios" --patch-dir ./patches/react-native-pdf
23+
```
24+
25+
This will create a patch file in the `patches/` directory. After creating the patch:
26+
27+
- Create or update the `details.md` file in the library's patch directory
28+
- Document the patch according to the [`details.md` format](#-detailsmd-format)
29+
30+
> 📝 **Note**
31+
>
32+
> The `--patch-dir` option should only be specified when the target library has a dedicated subdirectory within the `./patches` directory.
33+
> If no such subdirectory exists, omit this option from the command.
34+
35+
---
36+
937
## 🗂️ Folder Structure
1038

1139
Each library with patch (or patches) should have its own directory inside `patches/`. Each directory should contain:
@@ -54,23 +82,34 @@ Each patch must be listed and explained in the `details.md` file within the same
5482

5583
### [<patch-name>.patch](<patch-name>.patch)
5684

57-
- Reason: <Why this patch is needed>
58-
- Upstream PR/issue: <link or 🛑 if not raised. If no upstream issue or PR exists, explain why>
59-
- E/App issue: <link or 🛑 if none>
60-
- PR Introducing Patch: <link to internal PR that added the patch>
85+
- Reason:
86+
87+
```
88+
Please explain why the patch is necessary
89+
```
90+
91+
- Upstream PR/issue: <Please create an upstream PR fixing the patch. Link it here and if no upstream issue or PR exists, explain why>
92+
- E/App issue: <Please create an E/App issue ([template](./../.github/ISSUE_TEMPLATE/NewPatchTemplate.md)) for each introduced patch. Link it here and if patch won't be removed in the future (no upstream PR exists) explain why>
93+
- PR introducing patch: <Link to E/App (or Mobile-Expensify) PR that added the patch>
6194
```
6295

6396
### Example
6497

6598
```md
6699
# `react-native-pdf` patches
67100

68-
### [react-native-pdf+6.7.3+002+fix-incorrect-decoding.patch](react-native-pdf+6.7.3+002+fix-incorrect-decoding.patch)
101+
### [react-native-pdf+6.7.3+001+update-podspec-to-support-new-arch.patch](react-native-pdf+6.7.3+001+update-podspec-to-support-new-arch.patch)
102+
103+
- Reason:
104+
105+
```
106+
This patch updates the react-native-pdf.podspec to ensure compatibility with React Native's New Architecture on iOS by replacing manual dependency declarations
107+
with Meta's recommended `install_modules_dependencies` function
108+
```
69109

70-
- Reason: If the file name contains accented characters, the PDF load fails.
71-
- Upstream PR/issue: 🛑
72-
- E/App issue: 🛑
73-
- PR Introducing Patch: [#50043](https://github.com/Expensify/App/pull/50043)
110+
- Upstream PR/issue: https://github.com/wonday/react-native-pdf/pull/803
111+
- E/App issue: 🛑 TODO
112+
- PR Introducing Patch: https://github.com/Expensify/App/pull/13767
74113
```
75114

76115
## ✅ Patch Submission Checklist
Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
# `react-native-pdf` patches
22

3-
### [react-native-pdf+6.7.3+001+initial.patch](react-native-pdf+6.7.3+001+initial.patch)
3+
### [react-native-pdf+6.7.3+001+update-podspec-to-support-new-arch.patch](react-native-pdf+6.7.3+001+update-podspec-to-support-new-arch.patch)
4+
5+
- Reason:
6+
7+
```
8+
This patch updates the react-native-pdf.podspec to ensure compatibility with React Native's New Architecture on iOS by replacing manual dependency declarations
9+
with Meta's recommended `install_modules_dependencies` function
10+
```
411
5-
- Reason: Explanation/details
612
- Upstream PR/issue: https://github.com/wonday/react-native-pdf/pull/803
713
- E/App issue: 🛑
8-
- PR Introducing Patch: [#13767](https://github.com/Expensify/App/pull/13767)
14+
- PR Introducing Patch: https://github.com/Expensify/App/pull/13767

patches/react-native-pdf/react-native-pdf+6.7.3+001+initial.patch renamed to patches/react-native-pdf/react-native-pdf+6.7.3+001+update-podspec-to-support-new-arch.patch

File renamed without changes.
Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,25 @@
11
# `react-native-tab-view` patches
22

3-
### [react-native-tab-view+4.1.0+001+initial.patch](react-native-tab-view+4.1.0+001+initial.patch)
3+
### [react-native-tab-view+4.1.0+001+fix-tab-animation.patch](react-native-tab-view+4.1.0+001+fix-tab-animation.patch)
44

5-
- Reason: Explanation/details
6-
- Upstream PR/issue: N/A
7-
- E/App issue: https://github.com/Expensify/App/issues/38796 | https://github.com/Expensify/App/issues/39285
5+
- Reason:
6+
7+
```
8+
This patch addresses an issue where the camera in IOURequestStepScan fails to render due to the current position value
9+
from react-native-tab-view not updating during tab transitions. It explicitly trigger a re-render when the active tab or page changes.
10+
```
11+
12+
- Upstream PR/issue: 🛑
13+
- E/App issue: 🛑
814
- PR Introducing Patch: [#39854](https://github.com/Expensify/App/pull/39854)
9-
- PR Updating Patch: [#60468](https://github.com/Expensify/App/pull/60468)
1015
1116
### [react-native-tab-view+4.1.0+002+fix-glitching-on-initial-load.patch](react-native-tab-view+4.1.0+002+fix-glitching-on-initial-load.patch)
1217
13-
- Reason: There was a visible glitching on initial load. The glitch was visible because two animated values: position and translateX were equal to 0 for a brief moment before changing to a proper value.
18+
- Reason:
19+
```
20+
There was a visible glitching on initial load. The glitch was visible because two animated values:
21+
position and translateX were equal to 0 for a brief moment before changing to a proper value.
22+
```
1423
- Upstream PR/issue: https://github.com/react-navigation/react-navigation/pull/12627#issuecomment-2945055209
1524
- E/App issue: https://github.com/Expensify/App/issues/62346
16-
- PR Introducing Patch: [#63570](https://github.com/Expensify/App/pull/63570)
25+
- PR Introducing Patch: [#63570](https://github.com/Expensify/App/pull/63570)

patches/react-native-tab-view/react-native-tab-view+4.1.0+001+initial.patch renamed to patches/react-native-tab-view/react-native-tab-view+4.1.0+001+fix-tab-animation.patch

File renamed without changes.

scripts/validatePatches.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ done
4848
if [[ ${#ERRORS[@]} -eq 0 ]]; then
4949
echo "✅ All changed patches are valid!"
5050
else
51+
printf "Refer to https://github.com/Expensify/App/blob/main/contributingGuides/PATCHES.md to fix following errors:\n"
5152
printf "%s\n" "${ERRORS[@]}"
5253
exit 1
5354
fi

0 commit comments

Comments
 (0)