[react] Properly type form-related events#74383
[react] Properly type form-related events#74383eps1lon merged 3 commits intoDefinitelyTyped:masterfrom
Conversation
| } | ||
| } | ||
|
|
||
| function formrelatedEventTests() { |
There was a problem hiding this comment.
Moved from the React tests. It makes more sense here since React tests don't have access to the full lib/dom.d.ts.
| // | ||
| // The SyntheticEvent.target.value should be accessible for onChange | ||
| // -------------------------------------------------------------------------- |
There was a problem hiding this comment.
The test wasn't actually testing it. Just that you can cast event.target up to a more specific EventTarget e.g. HTMLTextAreaElement. react-dom tests now assert in more detail when the target can be narrowed and not.
c63cd58 to
fc3de51
Compare
fc3de51 to
a79247a
Compare
| }; | ||
|
|
||
| class SynchronousValidationForm | ||
| extends React.Component<ReduxFormProps<SynchronousValidationForm & HTMLFormElement>> |
There was a problem hiding this comment.
This created a handleSubmit that assumes the onSubmit's target is an intersection of SynchronousValidationForm and HTMLFormElement. But the handleSubmit is passed to React's onSubmit which definitely won't provide that target. Looks like an oversight due to not properly typing onSubmit. Now we do so flushing out this seemingly invalid usage is expected.
|
@eps1lon Thank you for submitting this PR! This is a live comment that I will keep updated. 3 packages in this PR
Code ReviewsBecause this is a widely-used package, a DT maintainer will need to review it before it can be merged. You can test the changes of this PR in the Playground. Status
Once every item on this list is checked, I'll ask you for permission to merge and publish the changes. Diagnostic Information: What the bot saw about this PR{
"type": "info",
"now": "-",
"pr_number": 74383,
"author": "eps1lon",
"headCommitOid": "a79247a777886a820abc14d95d85c55a1df8cac1",
"mergeBaseOid": "e42909429872fbf1865e12e389edba88f426f978",
"lastPushDate": "2026-01-22T16:13:24.000Z",
"lastActivityDate": "2026-01-23T10:57:17.000Z",
"hasMergeConflict": false,
"isFirstContribution": false,
"tooManyFiles": false,
"hugeChange": false,
"popularityLevel": "Critical",
"pkgInfo": [
{
"name": "react-dom",
"kind": "edit",
"files": [
{
"path": "types/react-dom/test/react-dom-tests.tsx",
"kind": "test"
}
],
"owners": [
"MartynasZilinskas",
"theruther4d",
"Jessidhia",
"eps1lon"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
},
{
"name": "react",
"kind": "edit",
"files": [
{
"path": "types/react/global.d.ts",
"kind": "definition"
},
{
"path": "types/react/index.d.ts",
"kind": "definition"
},
{
"path": "types/react/test/index.ts",
"kind": "test"
},
{
"path": "types/react/ts5.0/global.d.ts",
"kind": "definition"
},
{
"path": "types/react/ts5.0/index.d.ts",
"kind": "definition"
},
{
"path": "types/react/ts5.0/test/index.ts",
"kind": "test"
}
],
"owners": [
"johnnyreilly",
"bbenezech",
"pzavolinsky",
"ericanderson",
"DovydasNavickas",
"theruther4d",
"guilhermehubner",
"ferdaber",
"jrakotoharisoa",
"pascaloliv",
"hotell",
"franklixuefei",
"Jessidhia",
"saranshkataria",
"lukyth",
"eps1lon",
"zieka",
"dancerphil",
"dimitropoulos",
"disjukr",
"vhfmag",
"priyanshurav",
"Semigradsky",
"mattpocock"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Critical"
},
{
"name": "redux-form",
"kind": "edit",
"files": [
{
"path": "types/redux-form/v4/redux-form-tests.tsx",
"kind": "test"
},
{
"path": "types/redux-form/v5/redux-form-tests.tsx",
"kind": "test"
}
],
"owners": [
"aikoven",
"LKay",
"bancek",
"huwmartin",
"m-b-davis",
"ethanresnick",
"maddijoyce",
"smifun",
"mshaaban088",
"esetnik",
"mrsekut",
"abemedia"
],
"addedOwners": [],
"deletedOwners": [],
"popularityLevel": "Popular"
}
],
"reviews": [],
"mainBotCommentID": 3789674041,
"ciResult": "pass"
} |
|
🔔 @MartynasZilinskas @theruther4d @Jessidhia @johnnyreilly @bbenezech @pzavolinsky @ericanderson @DovydasNavickas @guilhermehubner @ferdaber @jrakotoharisoa @pascaloliv @Hotell @franklixuefei @saranshkataria @lukyth @zieka @dancerphil @dimitropoulos @disjukr @vhfmag @priyanshurav @Semigradsky @mattpocock @aikoven @LKay @bancek @huwmartin @m-b-davis @ethanresnick @maddijoyce @smifun @mshaaban088 @esetnik @mrsekut @abemedia — please review this PR in the next few days. Be sure to explicitly select |
|
This broke our CI builds and I suspect 10 million other CI builds. I think we are all happy with this change but it was in fact marked as a patch version only and I don't see any changelog / migration docs? |
Sorry to hear that. This is an unfortunate reality with type changes. Any change is a breaking change if existing code was relying on these bugs. I made a judgement call that this breakage is fairly small considering what I saw in internal apps. If we'd hold off with every change until the next major, we'd rarely make progress. This one was a clear bug that should be fixed. Do you have some examples that broken? This could help other readers resolve their issues faster. |
Thanks, I appreciate your work and I totally understand as a senior dev for over 20 yrs I do agree you just have to push it forward, however I would have expected some release docs around it, and a minor version increment. It just required refactoring our types to use the new ones but as of just 1 year ago we had been using the recommended types from official docs so its likely more of a docs out of date issue than anything on your part. Thanks again for your work! |
This deprecates
React.FormEventwhich has no counterpart in the DOM and was equivalent toReact.SyntheticEvent. Instead we're now adding the correct corresponding events to form-related event handlers (change,input,invalid,reset,submit).As with any type change, this may cause type-checks to fail that were relying on bugs. We're only doing this change for the latest types (React 19) to reduce the blast radius. If the change is too disruptive, we may revert and defer to React 20.
This PR adds a second parameter to
ChangeEventspecifying thetarget. Change events could have bubbled up so for most elements you should just useChangeEvent<CurrentTarget>. Form-related elements likeinputalready useChangeEvent<CurrentTarget, HTMLInputElement>. In React 20, arbitraryonChangehandlers will not get the narrowedtarget(see #69436 (comment)).