lilactown / helix

A simple, easy to use library for React development in ClojureScript.
Eclipse Public License 2.0
624 stars 52 forks source link

helix.hooks: introduce use-id #126

Closed rome-user closed 1 year ago

rome-user commented 1 year ago

Motivation Although gensym already exist in Clojure, I find it useful to have this hook for two reasons:

So I went ahead and added the hook, together with a basic unit test for this hook and future wrappers over React hooks.

Caveats useId was introduced in React 18. This code bumps the minimum required React to version 18.

lilactown commented 1 year ago

This makes sense. I know some (e.g. my production app at work) is still on React 17; I imagine as long as we don't call this hook it will be fine.

rome-user commented 1 year ago

Some quick testing shows that indeed React 17 will simply cause use-id to be set to nil. I'm not sure if this is good behavior. I've considered three possibilities to handle the situation.

  1. Let it be possibly nil, and mention the minimum required version of React in the docstring.

    • Downside: users of React 17 (or older) who do not read the documentation will be surprised
  2. Introduce helix.hooks.modern namespace that contains wrappers for the latest hooks, slowly migrating them into helix.hooks as needed. The namespace's docstring will indicate this is where hooks introduced after React 17 exist.

    • Downside: if we move from helix.hooks.modern to helix.hooks, that requires refactoring. If we avoid this by making e.g. helix.hooks/use-id reference helix.hooks.modern/use-id, then we will have two ever-growing namespaces, users will likely get confused.
  3. Delay merging this PR until most CLJS users are on React 18.

    • Downside: we have to use react/useId for the time being, but this will provide better developer experience for users on older versions of React.

At work, I have an React app that is being migrated to Helix. It currently uses Reagent and React 17. I find a lot of value in lowering the barriers to migration as much as possible. For this reason, I think either 3 is the best approach here. What do you think?

lilactown commented 1 year ago

I think merging it is fine. We could provide a shim for it if it's not available. I'd have to play with the behavior and see how to reproduce it.