The Wayback Machine - https://web.archive.org/web/20201116073712/https://github.com/beautifulinteractions/beautiful-react-hooks/issues/162
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

Invalid `HandlerSetter` type for `useGlobalEvent` hook #162

Open
jacekk opened this issue Oct 3, 2020 · 3 comments
Open

Invalid `HandlerSetter` type for `useGlobalEvent` hook #162

jacekk opened this issue Oct 3, 2020 · 3 comments

Comments

@jacekk
Copy link

@jacekk jacekk commented Oct 3, 2020

Describe the bug
The following PR introduced some breaking change in typings: https://github.com/beautifulinteractions/beautiful-react-hooks/pull/156/files#diff-b52768974e6bc0faccb7d4b75b162c99R19

Argument of type '() => void' is not assignable to parameter of type 'any[]'.
  Type '() => void' is missing the following properties from type 'any[]': pop, push, concat, join, and 27 more.  TS2345

     7 | 	const onWindowResize = useGlobalEvent('resize');
     8 | 
  >  9 | 	onWindowResize(() => {
       | 	               ^
    10 | 		setWindowWidth(window.innerWidth);
    11 | 	});
    12 |

I've tried the following, but of course, it is not working properly. But satisfies the TS compiler 😸

onWindowResize([
	() => {
		setWindowWidth(window.innerWidth);
	},
]);

Possible workaround

Use the third param of useGlovalEvent hook:

useGlobalEvent('resize', undefined, () => {
	setWindowWidth(window.innerWidth);
});

To Reproduce
I've prepared CRA repo to show this bug.
One with latest version --> compare: failling-issue-v0.30.5
And one with version that introduced the problematic change: compare: failling-issue-v0.30.1

Expected behavior
Well, I should have no TS compiler errors following hook examples. I know, these are JS examples, but we should either provide working TS versions of examples, or fix the typings. I vote for the latter one.

Screenshots
N/A

Desktop (please complete the following information):
N/A

Smartphone (please complete the following information):
N/A

Additional context

$ node --version && npm --version && yarn --version
v12.11.1
6.11.3
1.22.5
@jacekk jacekk changed the title Invalid `HandlerSetter` type for `useGlobalEvent` hook. Invalid `HandlerSetter` type for `useGlobalEvent` hook Oct 3, 2020
@antonioru antonioru added the bug label Oct 5, 2020
@antonioru
Copy link
Member

@antonioru antonioru commented Oct 5, 2020

Hi @jacekk
thank you for pointing this out and yes this is definitely a bug urgently needs to be fixed
I merged that PR as I didn't notice the bug, I'm not great with TS

would you mind to open a PR?

@jacekk
Copy link
Author

@jacekk jacekk commented Oct 6, 2020

@antonioru I will try to find some time. Kinda busy days lately 😕

@antonioru
Copy link
Member

@antonioru antonioru commented Oct 6, 2020

@antonioru I will try to find some time. Kinda busy days lately 😕

hehe I totally understand :)
don't worry, no pressure meant, your work will be welcome as soon as you feel like doing it

thank you very much, I really appreciate it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.