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

useDefault is treated as a React hook unintentionally #80

Closed
naruaway opened this issue Aug 8, 2023 · 4 comments
Closed

useDefault is treated as a React hook unintentionally #80

naruaway opened this issue Aug 8, 2023 · 4 comments
Assignees
Labels
enhancement New feature or request priority This has priority

Comments

@naruaway
Copy link
Contributor

naruaway commented Aug 8, 2023

When using Valibot in a React project, as demonstrated by this StackBlitz example,
Just using useDefault at top-level like const schema = useDefault(string(), 'test') causes the following ESLint error:

 3:16  error  React Hook "useDefault" cannot be called at the top level. React Hooks must be called in a React function component or a custom React Hook function  react-hooks/rules-of-hooks

This error is caused by eslint-plugin-react-hooks, which is maintained by the React team and recommended officially in the doc.
Basically every well maintained React projects should have installed this ESLint rule.

Semantically, useSomeIdentifierHere is considered as a React hook by its naming convention and tools like the ESLint rule is following this definition. Note that this is not only about ESLint, other tools such as Fast Refresh implementation is also relying on this naming convention. People might be confused since useDefault looks like a React hook as well.

Usually it's weird to be constrained by "just a single framework" such as React but React is used extremely a lot and it would be nice if Valibot can be a good companion of React.

I searched Valibot repo and did the following Ripgrep search:

$ valibot/library rg  'use[A-Z]'
src/methods/index.ts
14:export * from './useDefault/index.ts';

src/methods/useDefault/index.ts
1:export * from './useDefault.ts';

src/methods/useDefault/useDefault.test.ts
4:import { useDefault } from './useDefault.ts';
6:describe('useDefault', () => {
8:    const schema1 = useDefault(string(), 'test');

src/methods/useDefault/useDefault.ts
11:export function useDefault<TSchema extends BaseSchema | BaseSchemaAsync>(

So we only need to think about an alternative name of useDefault.
My suggestion so far is to rename it to something like withDefault but there might be a better name

@naruaway naruaway changed the title useDefault causes ESLint errors in React projects useDefault is treated as React hooks unintentionally Aug 8, 2023
@naruaway naruaway changed the title useDefault is treated as React hooks unintentionally useDefault is treated as a React hook unintentionally Aug 8, 2023
@fabian-hiller
Copy link
Owner

Thank you for the info. Since default is a reserved word, I had the choice between useDefault and withDefault. However, I had not thought of this problem back then. Maybe we can find some more alternatives. Otherwise it will probably change it to withDefault.

@fabian-hiller fabian-hiller self-assigned this Aug 8, 2023
@fabian-hiller fabian-hiller added enhancement New feature or request priority This has priority labels Aug 8, 2023
@FlorianDevPhynix
Copy link
Contributor

withDefault makes more sense to me. It describes what happens, a schema with a default value.

@fabian-hiller
Copy link
Owner

I think I will change it later or tomorrow and publish a new version.

@fabian-hiller
Copy link
Owner

useDefault is now deprecated and withDefault is available 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority This has priority
Projects
None yet
Development

No branches or pull requests

3 participants