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

[DEV] collection of useful react hooks #2002

Merged
merged 4 commits into from
Sep 21, 2023
Merged

[DEV] collection of useful react hooks #2002

merged 4 commits into from
Sep 21, 2023

Conversation

panaC
Copy link
Member

@panaC panaC commented Sep 21, 2023

No description provided.

@panaC panaC requested a review from danielweck September 21, 2023 11:49
@panaC panaC self-assigned this Sep 21, 2023
@socket-security
Copy link

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
use-sync-external-store 1.2.0 environment +0 38 kB gnoff

@panaC panaC merged commit 87d059c into develop Sep 21, 2023
4 checks passed
@panaC panaC deleted the feature/pierre/hooks branch September 21, 2023 12:40
React.useEffect(() => {
registerKeyboardListener(ListenForKeyUP, keyboardShortcut(keyboardShortcutState), callback);
return () => unregisterKeyboardListener(callback);
}, [keyboardShortcutState]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested this hook placeholder yet, but I think this will cause unnecessary renders due to the "state" object inside the dependency array being compared via reference identity. See keyboardShortcutsMatch() calls in componentDidUpdate() class component lifecycle.
I also don't see the ensureKeyboardListenerIsInstalled() call equivalent to the componentDidMount() class component lifecycle (React.useEffect() with empty dependency array)
@panaC did you test this hook, or is this work in progress? (the PR is marked as "[DEV]")
Thank you :)

@danielweck
Copy link
Member

What's the difference with #1997 ?

@panaC panaC restored the feature/pierre/hooks branch November 23, 2023 08:25
@panaC panaC deleted the feature/pierre/hooks branch November 23, 2023 08:28
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.

2 participants