Skip to content

Name unnamed parameters in function type annotations - #2169

Merged
meta-codesync[bot] merged 1 commit into
mainfrom
export-D129441819
Nov 4, 2026
Merged

meta-codesync[bot] merged 1 commit into
mainfrom
export-D129441819

Conversation

@SamChou19815

Copy link
Copy Markdown
Member

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

Gives every unnamed parameter in a Flow function type annotation a name: v for a lone parameter, v<index> otherwise, and rest for a rest parameter. Type-only change with no runtime behavior change.

Changelog: [Internal]

Reviewed By: panagosg7

Differential Revision: D129441819

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

Gives every unnamed parameter in a Flow function type annotation a name: `v` for a lone parameter, `v<index>` otherwise, and `rest` for a rest parameter. Type-only change with no runtime behavior change.

Changelog: [Internal]

Reviewed By: panagosg7

Differential Revision: D129441819
@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 D129441819.

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

Metro review: Looks good

Risk: low · Commit: 0fe892b · 9 commands run

This PR names previously unnamed parameters in Flow function types and has no runtime effect: all 61 changed JS files compile to identical Babel output at head and base. Flow, ESLint, Prettier, the API snapshots and tsc over the regenerated definitions are all clean. The claim of naming "every" parameter is slightly overstated, since packages/metro-file-map/src/crawlers/__tests__/node-test.js (lines 17 and 148) and flow-typed/npm/babel-types_v7.x.x.js still have unnamed ones; neither is in this diff and nothing fails because of them.

No findings.

Verified
  • ✅ yarn typecheck — Flow reported no errors at head.
  • ✅ node bench/strip-compare.js <head> <base> — Compiled all 61 changed JS files with the repo's Babel config, comments and whitespace removed: 61 identical, 0 different, 0 errors. The script is in the head checkout's untracked bench/ directory.
  • ✅ yarn eslint <changed files under packages, scripts, private> — No lint output. flow-typed/ is excluded by .eslintignore.
  • ✅ yarn prettier --check <61 changed JS files>
  • ✅ yarn build-ts-defs — Exit 0 at head. It printed unused-import lint messages for files including the unchanged worker.js; whether base prints the same was not compared. Generated definitions carry the new names, e.g. filter: (v: {...}).
  • ✅ yarn verify-api-snapshots — 18 snapshots verified, both before and after regenerating definitions at head.
  • ✅ yarn tsc --project tsconfig.json — No errors reported over the regenerated definitions.
  • ✅ yarn jest --no-watchman packages/metro-runtime packages/metro-file-map/src/lib packages/metro/src/lib packages/metro/src/__tests__/HmrServer-test.js — 31 suites, 751 tests passed. Jest warned that a worker failed to exit gracefully.
  • ✅ node unnamed.js <head> — Informational AST scan: 6 unnamed parameters remain in 2 files outside the diff. 78 of 585 tracked files failed to parse and were not scanned, so the count is a lower bound.

Not run: No benchmarks: the compiled output is identical, so runtime cost can't change. The full Jest suite was not run, only the touched-area subset, because identical compiled output makes wider runs low value. Definitions were not generated at base or diffed against head: the session's permission check auto-denied the compound command that did this, and I did not retry that step, so the check relies on the head snapshot verification and tsc. Windows was not tested; the change is type-only.

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

@meta-codesync
meta-codesync Bot merged commit 55d00f5 into main Nov 4, 2026
18 of 19 checks passed
@SamChou19815
SamChou19815 deleted the export-D129441819 branch November 4, 2026 20:36
@robhogan

robhogan commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

That's.. imaginative 😅

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 needs-meta-import

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants