fix: standardize updateApp to use undefined as no-change sentinel#75
Merged
kitsuyui merged 1 commit intoJul 11, 2026
Merged
Conversation
gh-counterPR gate
Repo dashboard
Reported by gh-counter |
Code Metrics Report
Details | | main (f26c7d4) | #75 (16afc11) | +/- |
|---------------------|----------------|---------------|------|
| Coverage | 41.1% | 41.1% | 0.0% |
| Files | 146 | 146 | 0 |
| Lines | 2578 | 2578 | 0 |
| Covered | 1061 | 1061 | 0 |
+ | Code to Test Ratio | 1:0.3 | 1:0.3 | +0.0 |
| Code | 13392 | 13389 | -3 |
+ | Test | 4296 | 4299 | +3 |
+ | Test Execution Time | 7s | 5s | -2s |Code coverage of files in pull request scope (100.0% → 100.0%)
Reported by octocov |
Previously _name and _icon defaulted to null and used falsy fallback (||), while _url defaulted to undefined and used a strict check (!== undefined). This asymmetry meant null behaved differently per parameter: it kept the existing value for name/icon but silently cleared the URL. Change all three to default to undefined and use !== undefined checks so null is treated as an explicit value for every parameter. Update the two test cases that relied on null as a no-change signal for name/icon.
kitsuyui
force-pushed
the
fix/audit-implicit-behavior-updateapp-null-url-clears-001
branch
from
40e649f to
3e448d5
Compare
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
1 participant
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.
Summary
updateApphad inconsistent sentinel semantics across its optional parameters:_namenull||(falsy)_urlundefined!== undefined(strict)_iconnull||(falsy)Callers reading only the signature could not tell whether passing
nullwould clear a field or fall back to the existing value. The asymmetry meantupdateApp(id, null, null, null)would silently clear the URL while leaving name and icon unchanged.Change
Standardize all three parameters to
undefinedas the no-change sentinel, using strict!== undefinedchecks throughout:undefined→ keep the existing value (no change)null→ replace the fieldUpdate the two test cases that passed
nullfor name or icon intending no-change — those now correctly useundefined.Behavior
No change to the URL semantics:
nullstill explicitly clears the URL (the test'keeps explicit null URLs when updating apps'continues to pass). The change makes name and icon consistent with that behavior.Verification
bun run lint: cleanbun run typecheck: no errorsbun run test: 244/244 passedtsc --noEmit: no errors