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

useState returns a new callback instance each time #496

Open
steinybot opened this issue Aug 1, 2021 · 5 comments
Open

useState returns a new callback instance each time #496

steinybot opened this issue Aug 1, 2021 · 5 comments
Labels
Milestone

Comments

@steinybot
Copy link
Contributor

steinybot commented Aug 1, 2021

Hooks.useState is returning a new SetStateHookCallback instance each time which causes comparison problems.

In vanilla React (sandbox):

import "./styles.css";
import React, { useEffect, useState } from 'react';

export default function App() {
  console.log("Rendering")
  const stateResult = useState(0);
  const count = stateResult[0];
  const setCount = stateResult[1];
  useEffect(() => {
    console.log("count changed")
  }, [count]);
  useEffect(() => {
    console.log("setCount changed")
  }, [setCount]);
  return (
    <div className="App">
      <p>Count: {count}</p>
      <button onClick={() => setCount(c => c + 1)}>Increment</button>
    </div>
  );
}

Clicking the button will change the count and so count changed will be logged each time.

In Slinky (repo):

package hello.world

import org.scalajs.dom.console
import slinky.core._
import slinky.core.annotations.react
import slinky.core.facade.Hooks.{useEffect, useState}
import slinky.web.html._

import scala.scalajs.js
import scala.scalajs.js.annotation.JSImport

@JSImport("resources/App.css", JSImport.Default)
@js.native
object AppCSS extends js.Object

@JSImport("resources/logo.svg", JSImport.Default)
@js.native
object ReactLogo extends js.Object

@react object App {
  type Props = Unit

  private val css = AppCSS

  val component = FunctionalComponent[Props] { _ =>
    console.log("Rendering")
    val (count, setCount) = useState(0)
    useEffect(() => {
      console.log("count changed")
    }, List(count))
    useEffect(() => {
      console.log("setCount changed")
    }, List(setCount))
    div(className:="App")(
      p(s"Count: $count"),
      button(onClick:=(() => setCount(c => c + 1)))("Increment")
    )
  }
}

Clicking the button will change the count and setCount and so both count changed and setCount changed will be logged each time.

@steinybot
Copy link
Contributor Author

You could say to leave out setCount from the dependencies since it should never change but that is a special case that I do not think is a good habit to get into. If you were to do that in JS then exhaustive-deps would complain and so it is better to include it.

@steinybot
Copy link
Contributor Author

What I don't understand is that even memoizing the callback doesn't work. Even though it is only ever created once, React thinks it has changed:

package hello.world

import hello.world.SafeHooks.useStateSafely
import org.scalajs.dom.console
import slinky.core._
import slinky.core.annotations.react
import slinky.core.facade.Hooks.{useEffect, useMemo}
import slinky.core.facade.SetStateHookCallback
import slinky.web.html._

import scala.annotation.unused
import scala.scalajs.js
import scala.scalajs.js.annotation.JSImport
import scala.scalajs.js.|

@JSImport("resources/App.css", JSImport.Default)
@js.native
object AppCSS extends js.Object

@JSImport("resources/logo.svg", JSImport.Default)
@js.native
object ReactLogo extends js.Object

object SafeHooks {
  @inline def useStateSafely[T](default: T): (T, SetStateHookCallback[T]) = {
    val call = SafeHooksRaw.useState[T](default)
    val callback = useMemo(() => {
      console.log("Creating new memoized callback")
      new SetStateHookCallback[T](call._2)
    }, List(call._2))
    (call._1, callback)
  }
}

@js.native
@JSImport("react", JSImport.Namespace, "React")
private[world] object SafeHooksRaw extends js.Object {
  def useState[T](@unused default: T | js.Function0[T]): js.Tuple2[T, js.Function1[js.Any, Unit]] = js.native
}

@react object App {
  type Props = Unit

  private val css = AppCSS

  val component = FunctionalComponent[Props] { _ =>
    console.log("Rendering")
    val (count, setCount) = useStateSafely(0)
    useEffect(() => {
      console.log("count changed")
    }, List(count))
    useEffect(() => {
      console.log("setCount changed")
    }, List(setCount))
    div(className:="App")(
      p(s"Count: $count"),
      button(onClick:=(() => setCount(c => c + 1)))("Increment")
    )
  }
}

Creating new memoized callback is logged once and yet setCount changed is logged each time the button is clicked.

@steinybot
Copy link
Contributor Author

steinybot commented Aug 2, 2021

Finally figured it out. It is because SetStateHookCallback is not a non-native JS type.

This works:

package hello.world

import hello.world.SafeHooks.useStateSafely
import org.scalajs.dom.console
import slinky.core._
import slinky.core.annotations.react
import slinky.core.facade.Hooks.{useEffect, useMemo}
import slinky.web.html._

import scala.annotation.unused
import scala.language.implicitConversions
import scala.scalajs.js
import scala.scalajs.js.annotation.{JSImport, JSName}
import scala.scalajs.js.|

@JSImport("resources/App.css", JSImport.Default)
@js.native
object AppCSS extends js.Object

@JSImport("resources/logo.svg", JSImport.Default)
@js.native
object ReactLogo extends js.Object

@JSImport("resources/uniqueId.js", JSImport.Default)
@js.native
object UniqueId extends js.Object

final class JsSetStateHookCallback[T](private val origFunction: js.Function1[js.Any, Unit]) extends js.Object {
  @JSName("setState")
  @inline def apply(newState: T): Unit =
    origFunction.apply(newState.asInstanceOf[js.Any])

  @JSName("transformState")
  @inline def apply(transformState: T => T): Unit =
    origFunction.apply(transformState: js.Function1[T, T])
}

object JsSetStateHookCallback {
  @inline implicit def toFunction[T](callback: JsSetStateHookCallback[T]): T => Unit = callback(_)

  @inline implicit def toTransformFunction[T](callback: JsSetStateHookCallback[T]): (T => T) => Unit = callback(_)
}

object SafeHooks {
  @inline def useStateSafely[T](default: T): (T, JsSetStateHookCallback[T]) = {
    val call = SafeHooksRaw.useState[T](default)
    val callback = useMemo(() => {
      console.log("Creating new memoized callback")
      new JsSetStateHookCallback[T](call._2)
    }, List(call._2))
    (call._1, callback)
  }
}

@js.native
@JSImport("react", JSImport.Namespace, "React")
private[world] object SafeHooksRaw extends js.Object {
  def useState[T](@unused default: T | js.Function0[T]): js.Tuple2[T, js.Function1[js.Any, Unit]] = js.native
}

@react object App {
  type Props = Unit

  private val css = AppCSS

  private val uniqueId = UniqueId

  @js.native
  trait WithUniqueId extends js.Object {
    def uniqueId: Int = js.native
  }

  implicit def withUniqueId(a: Any): WithUniqueId = a.asInstanceOf[WithUniqueId]

  val component = FunctionalComponent[Props] { _ =>
    console.log("Rendering")
    val (count, setCount) = useStateSafely(0)
    console.log(s"setCount id: ${setCount.uniqueId} ${setCount.uniqueId}")
    useEffect(() => {
      console.log("count changed")
    }, List(count))
    useEffect(() => {
      console.log(s"setCount changed: ${setCount.uniqueId} ${setCount.uniqueId}")
    }, List(setCount))
    div(className:="App")(
      p(s"Count: $count"),
      button(onClick:=(() => setCount(c => c + 1)))("Increment"),
      button(onClick:=(() => throw new RuntimeException("Boom")))("Crash")
    )
  }
}

With uniqueId.js for debugging:

(function() {
  let id_counter = 1;
  Object.defineProperty(Object.prototype, "__uniqueId", {
    writable: true
  });
  Object.defineProperty(Object.prototype, "uniqueId", {
    get: function() {
      if (typeof this.__uniqueId === "undefined")
        this.__uniqueId = id_counter++;
      return this.__uniqueId;
    }
  });
}());

@steinybot
Copy link
Contributor Author

Is there something funny going on with AnyVal? I would have thought that since origFunction is a js.Function1 which has a JS representation that there wouldn't be an object allocated for SetStateHookCallback and it would be treated as a JS function. Looking in the debugger that doesn't seem to be true, it is showing up as a facade type.

@shadaj
Copy link
Owner

shadaj commented Aug 3, 2021

Ah, this is a good catch! I think the reason AnyVal doesn't work is because Scala.js needs to support instance checks at runtime. I think the solution would be to use extension methods instead of immediately wrapping the callback.

@shadaj shadaj added the bug label Jan 30, 2022
@shadaj shadaj added this to the Backlog milestone Jan 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants