Repository navigation
[No QA] fix: update Knip for Bun shell interpolation - #100329
roryabraham merged 1 commit into
Conversation
|
|
|
@MelvinBot create a GitHub issue for this PR and assign @chrispader to it. Update this PR's Use this issue title:
Use this issue description: ProblemKnip 6.14.0 parses each static fragment of an interpolated Bun shell template as a standalone command. In benchmarking PR #98144, the command This is the same parsing limitation reported in webpro-nl/knip#1867. It blocks the benchmarking PR from using Bun shell naturally unless command arguments are incorrectly added to SolutionUpdate Knip to 6.34.0 in PR #100329. Knip fixed interpolated Bun shell parsing in version 6.25.0 by reconstructing the entire tagged template before extracting binaries, and the current release includes that fix. The update also recognizes Upstream fix: webpro-nl/knip@9422d06 |
|
Created #100330 — "Knip misidentifies Bun shell arguments as binaries" — and assigned I also updated this PR's I found no existing Knip-related issue covering this, so no duplicate was filed. |
Codecov Report✅ All modified and coverable lines are covered by tests. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
🚧 roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.4.73-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.4.73-3 🚀
Bundle Size Analysis (Sentry): |
cc @roryabraham
Knip 6.14.0 parses each static fragment of an interpolated Bun shell template as a standalone command. In the benchmarking PR #98144, this makes
apiin$`${ghExecutable} api user --jq .login`look like an undeclared executable and fails the Knip workflow. This PR updates Knip to 6.34.0, which includes the upstream interpolation fix, so the benchmarking PR can use Bun shell without adding argument names toignoreBinaries.Explanation of Change
Knip fixed interpolated Bun shell parsing in version 6.25.0 by reconstructing the entire tagged template before extracting binaries. This change updates Knip from 6.14.0 to 6.34.0 and refreshes the lockfile. The newer version also recognizes
ghas a system executable, so the benchmarking PR only needs to ignore binaries that Knip cannot identify itself.Upstream context: webpro-nl/knip#1867 and the fixing commit.
Fixed Issues
$ #100330
PROPOSAL: #100330
Tests
KNIP=true nlx knip@6.14.0 --include binaries --reporter compact --no-exit-code.scripts/bootstrapForDevice.ts: apias an unlisted binary.KNIP=true nlx knip@6.34.0 --include binaries --reporter compact --no-exit-code.apias a binary.nr knipand verify it completes without new binary findings.Offline tests
Not applicable. This changes a static development dependency used by local and CI analysis and does not change app network behavior.
QA Steps
This cannot be tested in staging or production. The PR changes development tooling only and does not affect app code or runtime behavior.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
No screenshots or videos are included because this PR only updates a development dependency and its lockfile. It does not change the app UI or runtime behavior.
Android: Native
Not applicable.
Android: mWeb Chrome
Not applicable.
iOS: Native
Not applicable.
iOS: mWeb Safari
Not applicable.
MacOS: Chrome / Safari
Not applicable.