react-admin
react-admin copied to clipboard
marmelab
`useRecordContext` may return `undefined` (v4.0.1)
What you were expecting:
useRecordContext returns RaRecord, so it can be used without undefined check
What happened instead: Error was shown, because record data was undefined
Steps to reproduce:
Use typescript and the useRecordContext hook. The typing doesn't show the return type can be also undefined
Other information: This PR recently fixed this for version 3.19.12 but it seems like this didn't make it into a 4.x release. This issue is also addressing the issue.
Environment
- React-admin version: 4.0.1
- React version: 17.0.2
Hi, Thank your for submitting this! Indeed we need to port this improvement to v4 as well. Would you like to make a PR for this?
Hi @slax57 ,
thanks for the quick answer. After having a more closer look into the source code, it seems like the change from https://github.com/marmelab/react-admin/pull/7495 actually made it into the 4.x version, but since there was no release for ra-core lately, this is not yet reflected in the ra-core/dist/index.d.ts file.
So in my opinion there doesn't need to be any more code changes, but rather just a new release (for ra-core), which builds the ra-core/dist/index.d.ts again with new type definitions.
Okay, so actually this has been ported to v4 already by @WiXSL with #7498 , and it's actually there since the v4.0.0.
What I can't figure out though is why the index.d.ts file is not up to date. I'll have to investigate.
Thanks for pointing this out!
So after investigation, it appears tsc won't include the undefined in the dts file if we don't set the strictNullChecks to true. We definitely should and will but I can't give you any ETA as we have more urgent tasks to finish.
If you want to help us make tsc happy, you can activate the strict mode locally, fix a few of the errors and make some PRs (without the tsconfig change for now).
Thanks again for reporting this!
Wish I had time to contribute to the strict work - but happy to see this ticket here and an explanation, ran across this issue tonight.
Changing to const context = useContext<RecordType|undefined>(RecordContext); could solve issue with useRecordContext in https://github.com/marmelab/react-admin/blob/master/packages/ra-core/src/controller/record/useRecordContext.ts
This issue is not fixed and is a known issue that will not be fixed any time soon. The distributed type files are set to ignore null and undefined, so there is currently no way to safely rely on the return types of hooks. See #9622
The fix in #7498 is correct if the types are built using strict mode, but because the types are built without strict null checks, adding | undefined to the type does nothing.
