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

init: ScalaJS (Tyrian) web client #55

Merged
merged 10 commits into from Mar 7, 2022
Merged

init: ScalaJS (Tyrian) web client #55

merged 10 commits into from Mar 7, 2022

Conversation

gvolpe
Copy link
Owner

@gvolpe gvolpe commented Feb 21, 2022

Besides adding a new webapp written in Scala, all the extensions and instances from Ciris in the domain were moved to different files due to not supporting ScalaJS. This way we can reuse all the domain datatypes from the ScalaJS app.

@gvolpe gvolpe marked this pull request as ready for review February 22, 2022 16:06
@gvolpe
Copy link
Owner Author

gvolpe commented Feb 22, 2022

@davesmith00000 any chances you can give this a quick look? I added a custom WebSocket client with a keep-alive support that I think could potentially make it upstream (keep-alive as an optional argument). WDYT?

Copy link

@davesmith00000 davesmith00000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I've left a couple of small comments.

Let me know if I can help further. If a more elegant solution to refocus input part doesn't present itself, I can try checking out the code and seeing if I can do anything with it.

Thanks for giving Tyrian a shot!

@gvolpe
Copy link
Owner Author

gvolpe commented Feb 23, 2022

Thanks for reviewing @davesmith00000 ! I have more to say about the custom WS client, will do when I get back home in a few hours!

project/Dependencies.scala Outdated Show resolved Hide resolved
@davesmith00000
Copy link

I'll have a look the those issues I've raised against Tyrian tonight, I'll also review the PR you sent over - Thanks!

@davesmith00000
Copy link

Focus cmd ticket: PurpleKingdomGames/tyrian#56

@davesmith00000
Copy link

If you can hang on, I'll try and merge all the tyrian PRs (got one more to do, plus anything else we can think of) and release them for you.

@gvolpe
Copy link
Owner Author

gvolpe commented Feb 23, 2022

Sure @davesmith00000 , I don't plan to merge this for now. On top of everything, now I'm getting link issues when running webapp/fastOptJS so I really need to figure out what's going on first 😄

@davesmith00000
Copy link

I'm in the process of trying to decide how much more I do before cutting another release, but all the issues you've uncovered should now be resolved in the repo. Since you're not merging this yet, you could try a local publish and see if it's better / if there's anything else that you need?

@gvolpe
Copy link
Owner Author

gvolpe commented Feb 27, 2022

@davesmith00000 sounds good, let me give it a try now that I have about one hour of free time :)

@gvolpe
Copy link
Owner Author

gvolpe commented Feb 27, 2022

@davesmith00000 all looks great, thanks a lot for all the improvements so quickly, you rock! 🤘🏽

Here's the diff with the locally published version of Tyrian.

diff --git a/modules/ws-client/src/main/scala/trading/client/Model.scala b/modules/ws-client/src/main/scala/trading/client/Model.scala
index 6bee3b3..fee3275 100644
--- a/modules/ws-client/src/main/scala/trading/client/Model.scala
+++ b/modules/ws-client/src/main/scala/trading/client/Model.scala
@@ -3,13 +3,18 @@ package trading.client
 import trading.domain.*
 import trading.ws.WsOut
 
+import tyrian.websocket.WebSocket
+
 enum WsMsg:
   case Error(msg: String)
+  case Connecting
+  case Connected(ws: WebSocket)
   case Disconnected
 
+  def asMsg: Msg = Msg.ConnStatus(this)
+
 enum Msg:
   case CloseAlerts
-  case Connect
   case SymbolChanged(input: String)
   case Subscribe
   case Unsubscribe(symbol: Symbol)
@@ -20,7 +25,7 @@ enum Msg:
 case class Model(
     symbol: Symbol,
     input: String,
-    ws: Option[WS],
+    ws: Option[WebSocket],
     wsUrl: String,
     socketId: Option[SocketId],
     onlineUsers: Int,
diff --git a/modules/ws-client/src/main/scala/trading/client/Subscriptions.scala b/modules/ws-client/src/main/scala/trading/client/Subscriptions.scala
index 6b7c585..b456530 100644
--- a/modules/ws-client/src/main/scala/trading/client/Subscriptions.scala
+++ b/modules/ws-client/src/main/scala/trading/client/Subscriptions.scala
@@ -5,9 +5,10 @@ import trading.ws.WsOut
 import io.circe.parser.decode as jsonDecode
 
 import tyrian.*
+import tyrian.websocket.WebSocket
 import tyrian.websocket.WebSocketEvent as WSEvent
 
-def wsSub(ws: Option[WS]): Sub[Msg] =
+def wsSub(ws: Option[WebSocket]): Sub[Msg] =
   ws.fold(Sub.emit(Msg.NoOp)) {
     _.subscribe {
       case WSEvent.Receive(str) =>
@@ -22,8 +23,8 @@ def wsSub(ws: Option[WS]): Sub[Msg] =
       case WSEvent.Open =>
         println("WS socket opened")
         Msg.NoOp
-      case WSEvent.Close =>
-        println("WS socket closed")
+      case WSEvent.Close(code, reason) =>
+        println(s"WS socket closed. Code: $code, reason: $reason")
         Msg.ConnStatus(WsMsg.Disconnected)
     }
   }
diff --git a/modules/ws-client/src/main/scala/trading/client/Update.scala b/modules/ws-client/src/main/scala/trading/client/Update.scala
index 78a7083..5bbc1cb 100644
--- a/modules/ws-client/src/main/scala/trading/client/Update.scala
+++ b/modules/ws-client/src/main/scala/trading/client/Update.scala
@@ -8,6 +8,7 @@ import io.circe.syntax.*
 
 import org.scalajs.dom
 import tyrian.*
+import tyrian.websocket.{ KeepAliveSettings, WebSocket }
 
 def disconnected(model: Model): (Model, Cmd[Msg]) =
   model.copy(error = Some("Disconnected from server, please click on Connect.")) -> Cmd.Empty
@@ -22,12 +23,24 @@ def runUpdates(msg: Msg, model: Model): (Model, Cmd[Msg]) =
     case Msg.NoOp =>
       model -> Cmd.Empty
 
-    case Msg.Connect =>
-      WS.connect(model.wsUrl) match
-        case Left(cause) =>
-          model -> Cmd.Emit(Msg.ConnStatus(WsMsg.Error(cause)))
-        case Right(ws) =>
-          model.copy(error = None, ws = Some(ws)) -> refocusInput
+    case Msg.ConnStatus(WsMsg.Connecting) =>
+      val cmd = Cmd.RunTask(
+        WebSocket.connect(model.wsUrl, KeepAliveSettings.default),
+        {
+          case Left(err) => WsMsg.Error(err).asMsg
+          case Right(ws) => WsMsg.Connected(ws).asMsg
+        }
+      )
+      model -> cmd
+
+    case Msg.ConnStatus(WsMsg.Connected(ws)) =>
+      model.copy(error = None, ws = Some(ws)) -> refocusInput
+
+    case Msg.ConnStatus(WsMsg.Disconnected) =>
+      model.copy(socketId = None) -> Cmd.Empty
+
+    case Msg.ConnStatus(WsMsg.Error(cause)) =>
+      model.copy(error = Some(s"Connection error: $cause")) -> Cmd.Empty
 
     case Msg.CloseAlerts =>
       model.copy(error = None, sub = None, unsub = None) -> refocusInput
@@ -68,9 +81,3 @@ def runUpdates(msg: Msg, model: Model): (Model, Cmd[Msg]) =
 
     case Msg.Recv(WsOut.Notification(t: Alert.TradeUpdate)) =>
       model.copy(tradingStatus = t.status) -> Cmd.Empty
-
-    case Msg.ConnStatus(WsMsg.Disconnected) =>
-      model.copy(socketId = None) -> Cmd.Empty
-
-    case Msg.ConnStatus(WsMsg.Error(cause)) =>
-      model.copy(error = Some(s"Connection error: $cause")) -> Cmd.Empty
diff --git a/modules/ws-client/src/main/scala/trading/client/View.scala b/modules/ws-client/src/main/scala/trading/client/View.scala
index c11bf8a..c76e191 100644
--- a/modules/ws-client/src/main/scala/trading/client/View.scala
+++ b/modules/ws-client/src/main/scala/trading/client/View.scala
@@ -9,9 +9,6 @@ import tyrian.*
 import tyrian.Html.*
 
 def render(model: Model): Html[Msg] =
-  val tableHidden: Attr[Nothing] =
-    if model.alerts.isEmpty then hidden else attribute("foo", "")
-
   div(`class` := "container")(
     genericErrorAlert(model),
     subscriptionSuccess(model),
@@ -25,7 +22,7 @@ def render(model: Model): Html[Msg] =
         placeholder := "Symbol (e.g. EURUSD)",
         onInput(s => Msg.SymbolChanged(s)),
         onKeyDown(subscribeOnEnter),
-        Property("value", model.input)
+        value := model.input
       ),
       div(`class` := "input-group-append")(
         button(
@@ -44,7 +41,7 @@ def render(model: Model): Html[Msg] =
       )
     ),
     p(),
-    table(`class` := "table table-inverse", tableHidden)(
+    table(`class` := "table table-inverse", hidden(model.alerts.isEmpty))(
       thead(
         tr(
           th("Symbol"),
@@ -70,7 +67,7 @@ def renderSocketId: Option[SocketId] => Html[Msg] =
     span(
       span(id := "socket-id", `class` := "badge badge-pill badge-secondary")(text("<Disconnected>")),
       span(text(" ")),
-      button(`class` := "badge badge-pill badge-primary", onClick(Msg.Connect))(text("Connect"))
+      button(`class` := "badge badge-pill badge-primary", onClick(WsMsg.Connecting.asMsg))(text("Connect"))
     )
 
 def renderTradeStatus: TradingStatus => Html[Msg] =
diff --git a/modules/ws-client/src/main/scala/trading/client/WS.scala b/modules/ws-client/src/main/scala/trading/client/WS.scala
deleted file mode 100644
index be68006..0000000
--- a/modules/ws-client/src/main/scala/trading/client/WS.scala
+++ /dev/null
@@ -1,89 +0,0 @@
-package trading.client
-
-import org.scalajs.dom
-import tyrian.{ Cmd, Sub, Task }
-import tyrian.websocket.WebSocketEvent
-import util.Functions
-
-// Adapted from tyrian.websocket.WebSocket
-final class WS(liveSocket: LiveSocket):
-  def publish[Msg](message: String): Cmd[Msg] =
-    Cmd.SideEffect(() => liveSocket.socket.send(message))
-
-  def subscribe[Msg](f: WebSocketEvent => Msg): Sub[Msg] =
-    if WebSocketReadyState.fromInt(liveSocket.socket.readyState).isOpen then liveSocket.subs.map(f)
-    else Sub.emit(f(WebSocketEvent.Close))
-
-final class LiveSocket(val socket: dom.WebSocket, val subs: Sub[WebSocketEvent])
-
-enum WebSocketReadyState derives CanEqual:
-  case CONNECTING, OPEN, CLOSING, CLOSED
-
-  def isOpen: Boolean =
-    this match
-      case CLOSED  => false
-      case CLOSING => false
-      case _       => true
-
-object WebSocketReadyState:
-  def fromInt(i: Int): WebSocketReadyState =
-    i match {
-      case 0 => CONNECTING
-      case 1 => OPEN
-      case 2 => CLOSING
-      case 3 => CLOSED
-      case _ => CLOSED
-    }
-
-object WS:
-  def connect(address: String): Either[String, WS] =
-    newConnection(address, None).map(WS(_))
-
-  def connect(address: String, onOpenMessage: String): Either[String, WS] =
-    newConnection(address, Option(onOpenMessage)).map(WS(_))
-
-  private def newConnection(
-      address: String,
-      onOpenSendMessage: Option[String],
-      withKeepAliveMessage: Option[String] = None
-  ): Either[String, LiveSocket] =
-    try {
-      val socket    = new dom.WebSocket(address)
-      val keepAlive = new KeepAlive(socket, withKeepAliveMessage)
-
-      val subs =
-        Sub.Batch(
-          Sub.fromEvent("message", socket) { e =>
-            Some(WebSocketEvent.Receive(e.asInstanceOf[dom.MessageEvent].data.toString))
-          },
-          Sub.fromEvent("error", socket) { _ =>
-            Some(WebSocketEvent.Error("Web socket connection error"))
-          },
-          Sub.fromEvent("close", socket) { e =>
-            keepAlive.cancel()
-            Some(WebSocketEvent.Close)
-          },
-          Sub.fromEvent("open", socket) { e =>
-            onOpenSendMessage.foreach(msg => socket.send(msg))
-            keepAlive.run()
-            Some(WebSocketEvent.Open)
-          }
-        )
-
-      Right(LiveSocket(socket, subs))
-    } catch {
-      case e: Throwable =>
-        Left(s"Error trying to set up websocket: ${e.getMessage}")
-    }
-
-  final class KeepAlive(socket: dom.WebSocket, msg: Option[String]):
-    private var timerId = 0;
-
-    def run(): Unit =
-      if socket != null && WebSocketReadyState.fromInt(socket.readyState).isOpen then
-        println("[info] Sending heartbeat 💓")
-        socket.send(msg.getOrElse("{ \"Heartbeat\": {} }"))
-      timerId = dom.window.setTimeout(Functions.fun0(() => run()), 20000)
-
-    def cancel(): Unit =
-      if (timerId <= 0) then dom.window.clearTimeout(timerId) else ()
diff --git a/modules/ws-client/src/main/scala/trading/client/ui/AlertsUI.scala b/modules/ws-client/src/main/scala/trading/client/ui/AlertsUI.scala
index f36c4d7..5790866 100644
--- a/modules/ws-client/src/main/scala/trading/client/ui/AlertsUI.scala
+++ b/modules/ws-client/src/main/scala/trading/client/ui/AlertsUI.scala
@@ -7,14 +7,10 @@ import tyrian.{ Attr, Html }
 import tyrian.Html.*
 
 private def mkAlert(property: Option[String], divId: String, status: String, message: String): Html[Msg] =
-  //FIXME: div [ hidden property.isEmpty ]
-  val alertHidden: Attr[Nothing] =
-    if property.isEmpty then hidden else attribute("foo", "")
-
   div(
     id      := divId,
     `class` := s"alert alert-$status fade show",
-    alertHidden
+    hidden(property.isEmpty)
   )(
     button(
       `class` := "close",
diff --git a/project/Dependencies.scala b/project/Dependencies.scala
index cc5af42..057e1ad 100644
--- a/project/Dependencies.scala
+++ b/project/Dependencies.scala
@@ -23,7 +23,7 @@ object Dependencies {
     val refined       = "0.9.28"
 
     val scalajsTime = "2.4.0-M1"
-    val tyrian      = "0.3.1"
+    val tyrian      = "0.3.2-SNAPSHOT"
 
     val scalacheck = "1.15.4"
     val weaver     = "0.7.10"

@gvolpe
Copy link
Owner Author

gvolpe commented Feb 27, 2022

@davesmith00000 since we're here, I forgot to comment that as a ScalaJS newbie, one of the instructions in the installation section got me puzzled for a couple of hours getting weird compilation / linking errors.

This line here:

scalaJSUseMainModuleInitializer := true

I don't know exactly what it means but most Tyrian examples don't work with this line. Do you think it should be removed from the documentation? If so, I can submit a quick PR to do so.

@davesmith00000
Copy link

Diff looks great! Did you try the Dom.focus command?

scalaJSUseMainModuleInitializer := true

You're right! It used to be needed but no longer (unless you're using scalajs-bunder, but that's another story). If you have time to update the docs that would be awesome, if not I can do it - lemme know! 😄

@gvolpe
Copy link
Owner Author

gvolpe commented Feb 27, 2022

Diff looks great! Did you try the Dom.focus command?

Oh no, I forgot about this one! Will do now.

You're right! It used to be needed but no longer (unless you're using scalajs-bunder, but that's another story). If you have time to update the docs that would be awesome, if not I can do it - lemme know! smile

Ah got it. Will submit a PR now then :)

@davesmith00000
Copy link

Tyrian 0.3.2 is out with the fixes you need in it. 😄

@gvolpe gvolpe merged commit d1236c5 into main Mar 7, 2022
@gvolpe gvolpe deleted the frontend/scalajs-tyrian branch March 7, 2022 11:29
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

Successfully merging this pull request may close these issues.

None yet

2 participants