Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix typing for parity with React #16

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kevinweber
Copy link

@kevinweber kevinweber commented Feb 9, 2022

Fixes in this PR get the typing closer to what React's testing library does and thereby makes migrations from React to Preact easier. When I migrated from React to Preact, a TS error appeared as soon as I imported renderHook from the Preact testing library:
Screen Shot 2022-02-02 at 3 53 47 PM

The error says that result.current could possibly be undefined, meaning I need to unfortunately refactor result.current(yo) to result.current?.(yo).

At first this issue makes sense because the resultContainer function has an initialValue argument that might be undefined and also the value of let value might be undefined. However, the library actually never calls resultContainer with an argument, justifying the removal of initialValue as a potential argument.

That doesn't solve the potential undefined value though. For this, I'm following the pattern in React's resultContainer by assigning value as R. This way types are safe.

I verified type-safety locally in one of the tests. When I change…

const { result } = renderHook(() => useCounter());
...
expect(result.current.count).toBe(1);

…to…

const { result } = renderHook(() => 'different');
...
expect(result.current.count).toBe(1);

…TS throws an error for result.current.count, saying that result.current can't have a property count – which is correct because result.current is now of type string.

@kevinweber kevinweber changed the title Fix typing for parity with React equivalent Fix typing for parity with React Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant