Skip to content

feat(useIsBrowser): add useIsBrowser hook #202

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

Closed

Conversation

JeongHwan-dev
Copy link
Contributor

Overview

Add useIsBrowser Hook: a simple hook that detects if code is running in browser environment.

Checklist

  • Did you write the test code?
  • Have you run yarn test:coverage to make sure there is no uncovered line?
  • Did you write the JSDoc?

@uniqueeest
Copy link
Contributor

Internally, it doesn't use any React hooks like useState or useEffect, just typeof window !== 'undefined', so it doesn't seem to be any different from a regular function.

Is there any reason why a function implemented like this needs to be a hook? It seems more natural to separate it into a simple utility, and I'm curious about the reasoning behind making it a hook.

@seungrodotlee
Copy link
Member

I also agree that this feature should be implemented as a utility function or a constant, rather than a hook.
Since it doesn’t manage any state, implementing and using it in the form of a hook seems somewhat inefficient.

@JeongHwan-dev
Copy link
Contributor Author

@uniqueeest, @seungrodotlee
I agree with both of your opinions. This hook doesn't utilize React's state or lifecycle methods and simply uses typeof window !== 'undefined', so I think it would be more appropriate to implement it as a utility function or constant rather than a hook.

The reason I initially implemented it as a hook was to maintain consistency with the usage patterns of other hooks in the library. I also thought that the hook format would be more familiar for users since it would primarily be used within React components. However, after giving it more thought, I agree that this functionality doesn't manage any state and only performs simple environment detection, so implementing it as a general utility would be more efficient in terms of resources. Therefore, I'll close this PR and consider submitting a new one with the implementation as a utility function or constant instead.

Thank you for your valuable feedback. 🙇🏻‍♂️

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.

3 participants