Remove unsafe Object.entries assertions in compoundParamsKey - #100554
Draft
KJ21-ENG wants to merge 1 commit into
Draft
Remove unsafe Object.entries assertions in compoundParamsKey#100554KJ21-ENG wants to merge 1 commit into
KJ21-ENG wants to merge 1 commit into
Conversation
Seatbelt-Operation: s07-cc439164-d770563-sign-1
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
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.
Explanation of Change
Remove both unsafe
Object.entriesinput assertions insrc/libs/compoundParamsKey.ts, the only changed file. Each existing result is now assigned toArray<[string, unknown]>before the unchanged chain. The compiler checks the already narrowed object input, and downstream values remainunknown. There are no support or test changes.No intentional runtime behavior change. The accepted plan and exact diff preserve guards, enumeration frequency, filter/map/sort order, signatures, exports and serialization. The existing Jest suite and direct production probes cover recursive key sorting, array order, top-level undefined omission, nested undefined placeholders, frozen inputs, own enumerable string keys, nonplain objects and existing serialization failures.
The encoder retains the route key plus NUL delimiter and normalized JSON suffix. Native focus restoration splits at NUL; web/native pruning uses the route-plus-NUL prefix. Probes cover NUL in parameter strings, exact placeholder bytes and the existing collision between nested undefined and its placeholder string. Empty object/array and scalar normalization equivalences also remain. Keys are not collision-free; route keys still assume no NUL. The probe records an out-of-contract NUL route without extending that contract.
src/libs/compoundParamsKey.tsNet pending warning reduction: 2; merged credit: 0.
config/eslint/eslint.seatbelt.tsvmatches the base, signed head and working tree. Read-only checks left it unchanged, so no restoration write was required; merge automation owns the eventual reduction.Fixed Issues
$ #94739
PROPOSAL: #94739 (comment)
Tests
Fresh independent verification passed all seven focused kinds: target-zero, ESLint, changed-root TypeScript, executable tests, formatting/spelling, changed paths and diff hygiene. Evidence is for the unchanged reviewed candidate:
SEATBELT_READ_ONLY=1 SEATBELT_DISABLE=1 eslint --no-cache --format json src/libs/compoundParamsKey.ts: zero findings or suppressed messages.checkChangedTypeScript(candidateRoot, 'tsconfig.json', ['src/libs/compoundParamsKey.ts']): complete authorized import-free target and applicable declarations, zero target/configuration diagnostics.jest --runTestsByPath tests/unit/compoundParamsKeyTest.ts --runInBand --no-cache --watch=falsewith private output/cache paths: 20/20 existing tests passed. Installed resolver and exact source-map contents identify the real candidate target. 11 additional direct production boundary groups passed, including bigint/cyclic failures, functions/symbols, Date/RegExp, frozen inputs, enumeration, normalization and collision behavior. These are fresh counts; older review probe counts are historical.oxfmt --check src/libs/compoundParamsKey.tsand one-file CSpell: passed; each checked the actual target.git diff --check HEADand exact changed-path checks passed. All 542 fresh recorded environment content hashes, five local packages, dependency link, Node runtime and TSV remained unchanged. Historical environment hashes remain unavailable.For repeatable local UI testing, perform the QA steps below in the development environment.
Offline tests
Not performed. On a previously loaded screen with a focusable input, disconnect the network, focus the input and move its caret, then navigate away and back through locally available history. Verify the expected screen and focus/caret restore without duplicate or stale history entries. Reconnect and repeat, checking for console errors. Run on web and native where the flow is available. This helper adds no network request or offline fallback.
QA Steps
Manual QA required; review first. This is production code used by router
resolveCursorForResetand web/native focus capture, restoration and pruning. The exact diff only adds typed bindings for the same two enumeration results; guards, enumeration, chain order, serialization, delimiter bytes and exports stay unchanged. Exact-diff inspection, the existing production Jest suite and fresh boundary probes support preserved branches and outcomes. UI execution remains outstanding.Proposed staging checks on supported web and native platforms:
PR Author Checklist
Checked items below acknowledge the linked issue and written repeatable test steps, not execution of the proposed UI checks. Offline, High Traffic, platform, console, consumer UI and authenticated/unauthenticated deeplink checks remain unperformed. Screenshots are unavailable. This diff adds no code pattern, comment, app copy, CSS, asset, generic UI component, Storybook change, message-editing logic, form layout, feature or bug fix; related conditional acknowledgements are left unchecked as inapplicable. Existing unit tests were reused, with additional private verification probes. No merge of
mainafter review is recorded.### 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
None captured. The change has no visual diff, and web/native UI QA remains outstanding.