japgolly/scalajs-react

Another way to define hook components

Opened this issue · 0 comments

nafg commented

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 #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