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

Another way to define hook components #1088

Open
nafg opened this issue Oct 24, 2023 · 0 comments
Open

Another way to define hook components #1088

nafg opened this issue Oct 24, 2023 · 0 comments

Comments

@nafg
Copy link

nafg commented Oct 24, 2023

I appreciate all the thought and effort that went into designing the new hooks API. It maximizes the admirable design goal of the library, to prevent runtime errors as much as possible.

However, after using it a bit I feel like it isn't the best ergonomically. Some observations that come to mind:

  • There are two APIs, "primary" and "secondary." They are sort of "overloaded" in some sense, which sometimes confuses IntelliJ, which is a big productivity loss.
  • Hooks are defined without a name. With Primary they never get a name. With Secondary the same name has to be "reinvented" on each usage, since further hooks that use it and the render method access it as a function parameter that has to be named to use it.
  • By the same token, you can't ctrl-click the hook to see all its usages, or ctrl-click a usage to see which hook it comes from. Instead, you have to count hooks mentally to keep track which is which.
  • If you delete a hook you have to update all the subsequent usages (for Primary, changing .hook<N> to .hook<N-1> for N>M, for Secondary deleting it from all the function parameters)

Also, #1087

In my own codebase I've started using the following approach instead. It's based on for comprehensions. It still prevents a lot of kinds of violations of the "Rule of Hooks", though perhaps not as many as the current API, but I find it a lot more ergonomic and readable.

Here is an example component:

      Hook.component[Props] { props =>
        for {
          error <- Hook.useSettable(Option.empty[String])
        } yield {
          val field =
            M.TextField(
              props.stateSnapshot.zoomStateL(editLens).toTagMod,
              _.label := props.label,
              _.fullWidth,
              _.variant.outlined,
              _.size.small,
              _.margin.dense,
              ^.onBlur ==> { (e: ReactEventFromInput) =>
                props.setFull(e.target.checkValidity()) >>
                  error.set(Some(e.target.validationMessage).filter(_.nonEmpty))
              },
              _.error := error.value.isDefined,
              _.helperText :=? error.value.map(vdomNodeFromString)
            )()(props.extraProps *)
          props.tooltip.toOption.foldRight[VdomElement](field)(M.Tooltip(_)(_))
        }
      }

Here is that component using the current API:

      ScalaFnComponent.withHooks[Props]
        .useState(Option.empty[String])
        .render { (props, error) =>
          val field =
            M.TextField(
              props.stateSnapshot.zoomStateL(editLens).toTagMod,
              _.label := props.label,
              _.fullWidth,
              _.variant.outlined,
              _.size.small,
              _.margin.dense,
              ^.onBlur ==> { (e: ReactEventFromInput) =>
                props.setFull(e.target.checkValidity()) >>
                  error.setState(Some(e.target.validationMessage).filter(_.nonEmpty))
              },
              _.error := error.value.isDefined,
              _.helperText :=? error.value.map(vdomNodeFromString)
            )()(props.extraProps *)
          props.tooltip.toOption.foldRight[VdomElement](field)(M.Tooltip(_)(_))
        }

Here is how it's defined:

object Hook {
  case class Thunk[+A] private[Hook](run: () => A) {
    def map[B](f: A => B): Thunk[B] = Thunk(() => f(run()))
    def flatMap[B](f: A => Thunk[B]): Thunk[B] = f(run())
  }

  implicit def custom[A](hook: CustomHook[Unit, A]): Thunk[A] =
    Thunk(() => hook.unsafeInit(()))

  def useSettable[A](initial: => A) = custom(chesednow.sjs.common.useSettable(initial))

  def useMemo[D: Reusability, A](deps: => D)(f: D => A): Thunk[Reusable[A]] =
    custom(Hooks.UseMemo(deps)(f))

  def component[P](f: P => Thunk[VdomNode])(implicit name: sourcecode.FullName) =
    ScalaFnComponent.withHooks[P]
      .render(f(_).run())
      .withDisplayName(name)

  def componentNamed[P](name: String)(f: P => Thunk[VdomNode]) =
    ScalaFnComponent.withHooks[P]
      .render(f(_).run())
      .withDisplayName(name)
}

Some notes:

  • It's very incomplete
  • withDisplayName is from ScalaFnComponent doesn't include displayName #1087
  • I happen to be using Settable rather than StateSnapshot. (Partially because it's more safely de/composable, being based on a "modify" function rather than a "set" function. I know you once told me that breaks Reusability, which I've only recently begun to learn, and don't fully understand this yet, but anyway it's besides the point of the overall approach
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

No branches or pull requests

1 participant