Chore: Add a CI script to check if patches are valid and still applied#15086
Open
Chore: Add a CI script to check if patches are valid and still applied#15086
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a Yarn patch validation script at Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI runner
participant Linter as Linter step
participant Script as checkYarnPatches.js
participant FS as Repository files
CI->>Linter: start linter phase (RUN_TESTS==1)
Linter->>Script: execute `yarn checkYarnPatches`
Script->>FS: read `package.json`
Script->>FS: read `yarn.lock`
Script->>Script: validate patch: resolutions exist in yarn.lock
alt validations pass
Script-->>Linter: exit 0 (success)
Linter-->>CI: continue
else validation fails
Script-->>Linter: exit non‑zero (error + messages)
Linter-->>CI: exit early with failure
end
Suggested labels
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
Bot
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/tools/checkYarnPatches.ts`:
- Around line 28-29: The code currently skips malformed patch entries when the
regex match (patchTarget.match(/^(.+)@npm:(.+)$/)) fails; instead, update the
logic in checkYarnPatches so that when match is falsy you record or throw an
explicit error describing the invalid patch format (include the offending
patchTarget and the expected "packageName@npm:version" pattern) and cause
validation to fail (e.g., push to an errors array or throw/exit) rather than
using continue; locate the code around the match usage and the loop that
iterates patchTarget to implement this validation failure path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ed69c7d4-cdf9-4648-bcac-53e91fe4756f
📒 Files selected for processing (4)
.eslintignore.github/scripts/run_ci.sh.gitignorepackages/tools/checkYarnPatches.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/tools/checkYarnPatches.ts (1)
32-33: Prefer template literals over+concatenation in error messages.Line 32-33 and Line 49-51 can be simplified to template literals for consistency and readability.
💡 Proposed change
if (!match) { errors.push( - `Invalid patch format for "${key}": "${patchTarget}" does not match ` + - 'expected pattern "packageName@npm:version" or "packageName@version".', + `Invalid patch format for "${key}": "${patchTarget}" does not match expected pattern "packageName@npm:version" or "packageName@version".`, ); continue; } @@ if (!yarnLock.includes(patchPattern)) { errors.push( - `Resolution "${key}" patches ${packageName}@${patchVersion}, ` + - 'but yarn.lock has no matching entry. The dependency was likely ' + - 'upgraded — update the patch to target the current version.', + `Resolution "${key}" patches ${packageName}@${patchVersion}, but yarn.lock has no matching entry. The dependency was likely upgraded — update the patch to target the current version.`, ); }Based on learnings: In all TypeScript files across the repository, prefer template literals (backticks) for string construction instead of string concatenation with +.
Also applies to: 49-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/tools/checkYarnPatches.ts` around lines 32 - 33, Replace the string concatenation used in the error messages with template literals: update the message that currently builds `"Invalid patch format for \"" + key + "\": \"" + patchTarget + "\" does not match ..."` to use a single backtick template string referencing key and patchTarget, and make the analogous change for the other occurrence around lines 49-51 so both error messages use template literals for readability and consistency (refer to the variables key and patchTarget and the surrounding error message construction in packages/tools/checkYarnPatches.ts).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/tools/checkYarnPatches.ts`:
- Around line 12-16: The JSON.parse result stored in packageJson is untyped;
annotate packageJson with an explicit type to prevent unsafe any propagation
(for example declare packageJson as { resolutions?: Record<string,string>; } or
a fuller PackageJson interface) before assigning JSON.parse(await readFile(...))
so accessing packageJson.resolutions is type-safe; update the declaration in
checkYarnPatches.ts for the packageJson symbol accordingly and keep the existing
resolutions: Record<string,string> = packageJson.resolutions ?? {} usage.
---
Nitpick comments:
In `@packages/tools/checkYarnPatches.ts`:
- Around line 32-33: Replace the string concatenation used in the error messages
with template literals: update the message that currently builds `"Invalid patch
format for \"" + key + "\": \"" + patchTarget + "\" does not match ..."` to use
a single backtick template string referencing key and patchTarget, and make the
analogous change for the other occurrence around lines 49-51 so both error
messages use template literals for readability and consistency (refer to the
variables key and patchTarget and the surrounding error message construction in
packages/tools/checkYarnPatches.ts).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7a8824ee-22bf-4602-b8a5-c704e8a58025
📒 Files selected for processing (2)
package.jsonpackages/tools/checkYarnPatches.ts
✅ Files skipped from review due to trivial changes (1)
- package.json
coderabbitai
Bot
added
web
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 116-118: Update the yarn patch entries so they target the actual
installed versions and remove redundant fixes: inspect the patch files
referenced by "react-native@0.81.6" and "react-native-paper@5.14.5" (the patch
files under .yarn/patches referenced from package.json), verify whether the
RCTDeviceInfo.mm landscape-dimension change and any other modifications are
already present in upstream RN 0.81.6 and RNPaper 5.14.5, and either remove
those redundant hunks from the patch files or regenerate the patches against the
current upstream tarballs; then update the package.json patch specifiers to
match the real upstream versions (react-native@npm:0.81.6 and
react-native-paper@npm:5.14.5) so patches apply only when custom diffs remain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 86c37d3a-9527-4e49-b691-f2edfd16c911
⛔ Files ignored due to path filters (2)
.yarn/patches/react-native-camera-npm-4.2.1-24b2600a7e.patchis excluded by!**/.yarn/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
package.jsonpackages/app-mobile/web/webpack.config.ts
💤 Files with no reviewable changes (1)
- packages/app-mobile/web/webpack.config.ts
coderabbitai
Bot
added
ci
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 28: The checker in checkYarnPatches.js currently only ensures a patch
LOCATOR is present in yarn.lock; update the script to also resolve each patch
locator to its referenced filesystem path and verify the file exists (use
fs.existsSync or fs.promises.access, expanding ~ to the user's home via
os.homedir()), and additionally enforce locator/path consistency by confirming
the locator’s expected filename (basename) matches the actual patch file name or
contents; on any missing or inconsistent patch, print a clear error with the
offending locator/path and exit non-zero so CI fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b949269b-5226-48c9-a95b-e99d7acd4605
⛔ Files ignored due to path filters (2)
.yarn/patches/react-native-npm-0.81.6-e0b80aa8aa.patchis excluded by!**/.yarn/**yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (1)
package.json
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The recent nanoid Renovate update broke the web app at runtime because a patch was no longer being applied. This is difficult to detect because it happens at runtime. So instead we'll have a script that check if patches are still being applied on CI (this should already fail in this PR since a number of patches are no longer valid)