Skip to content

Make the conversions of parseInt and isNaN explicit - #2168

Open
SamChou19815 wants to merge 1 commit into
mainfrom
export-D129644365
Open

SamChou19815 wants to merge 1 commit into
mainfrom
export-D129644365

Conversation

@SamChou19815

Copy link
Copy Markdown
Member

Summary:
X-link: react/react-native#59799

TypeScript declares parseInt(string: string, radix?: number) and isNaN(number: number), and Flow is adopting the same signatures. Calls that pass another type rely on the function's implicit conversion and start to error.

Wrap each such argument in String(...) (parseInt) or Number(...) (isNaN), the conversion the function already applies, so behavior doesn't change. The calls were found from Flow's errors and rewritten by a codemod. 8 arguments in 6 files in React Native and Metro.

Changelog: [Internal]

Differential Revision: D129644365

Summary:
X-link: react/react-native#59799

TypeScript declares `parseInt(string: string, radix?: number)` and `isNaN(number: number)`, and Flow is adopting the same signatures. Calls that pass another type rely on the function's implicit conversion and start to error.

Wrap each such argument in `String(...)` (`parseInt`) or `Number(...)` (`isNaN`), the conversion the function already applies, so behavior doesn't change. The calls were found from Flow's errors and rewritten by a codemod. 8 arguments in 6 files in React Native and Metro.

Changelog: [Internal]

Differential Revision: D129644365
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 4, 2026
@meta-codesync

meta-codesync Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

@SamChou19815 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D129644365.

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

Metro review: Looks good

Risk: low · Commit: 7dda567 · 3 commands run

This PR wraps the two parseInt arguments in symbolicateChromeTrace in String(...) to prepare for Flow's stricter signature. It looks good: parseInt already applies the same string conversion to these JSON-parsed values, and both are null-checked just above. Tests, lint and Flow pass, and the existing .cpuprofile fixture has funcVirtAddr values for this branch.

No findings.

Verified
  • ✅ yarn jest --no-watchman packages/metro-symbolicate — 6 suites, 77 tests and 33 snapshots passed. The testfile.cpuprofile fixture has string funcVirtAddr values.
  • ✅ yarn eslint packages/metro-symbolicate/src/Symbolication.js
  • ✅ yarn typecheck — Flow reported no errors with the repo's current Flow version, not the stricter signatures the PR anticipates.

Not run: No differential ran for the unchanged-behavior claim: the command for a script comparing parseInt(v, 10) with parseInt(String(v), 10) needed an approval this session can't give, so I didn't retry it. The claim rests on code reading, parseInt's specified conversion and fixture tests; only Symbol should differ, and JSON.parse can't produce one. No benchmark: one string conversion per trace frame in a CLI path. Windows: platform-neutral change. The stricter Flow signatures aren't in the repo's Flow version.

Automated review by Claude. Advisory only: it never approves or blocks, and a maintainer makes the call.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant