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

Fix warnings #322

Merged
merged 3 commits into from Jul 25, 2019
Merged

Fix warnings #322

merged 3 commits into from Jul 25, 2019

Conversation

zakpatterson
Copy link
Contributor

@zakpatterson zakpatterson commented Jul 25, 2019

Most of these warnings are monix deprecation warnings brought in by #320

A couple of them are a little suspect, as they appeared as discarded non-unit value warnings:

object EmitterBuilder {
  @inline def apply[E <: Event](eventType: String): CustomEmitterBuilder[E, VDomModifier] = ofModifier[E](obs => 
    Emitter(
      eventType, 
      event => {
        obs.onNext(event.asInstanceOf[E])
        ()
      }))

onNext creates a future, but I don't see any way that this future is waited on, as it's not returned or awaited. Not suggesting it should be awaited here, just unclear to me how this sideeffect is ok in this context.

Below are a bunch of warnings that have been fixed:

sbt:OutWatch> test
[info] Updating ...
[info] Done updating.
[warn] There may be incompatibilities among your library dependencies; run 'evicted' to see detailed eviction warnings.
[info] Compiling 30 Scala sources to /home/projects/oss/outwatch/outwatch/target/scala-2.12/classes ...
[warn] /home/projects/oss/outwatch/outwatch/src/main/scala/outwatch/dom/OutwatchAttributes.scala:28:33: discarded non-Unit value
[warn]     obs => f((o,p) => obs.onNext((o.elm.toOption, p.elm.toOption)))
[warn]                                 ^
[warn] /home/projects/oss/outwatch/outwatch/src/main/scala/outwatch/dom/helpers/EmitterBuilder.scala:37:152: discarded non-Unit value
[warn]   @inline def apply[E <: Event](eventType: String): CustomEmitterBuilder[E, VDomModifier] = ofModifier[E](obs => Emitter(eventType, event => obs.onNext(event.asInstanceOf[E])))
[warn]                                                                                                                                                        ^
[warn] /home/projects/oss/outwatch/outwatch/src/main/scala/outwatch/dom/helpers/EmitterBuilder.scala:6:33: Unused import
[warn] import monix.reactive.observers.Subscriber
[warn]                                 ^
[warn] /home/projects/oss/outwatch/outwatch/src/main/scala/outwatch/dom/helpers/Snabbdom.scala:3:28: Unused import
[warn] import monix.execution.Ack.Continue
[warn]                            ^
[warn] /home/projects/oss/outwatch/outwatch/src/main/scala/outwatch/util/Store.scala:49:74: method fromIO in trait ObservableDeprecatedBuilders is deprecated (since 3.0.0): Switch to Observable.from
[warn]       (newState, effect.fold[Observable[A]](Observable.empty)(Observable.fromIO))
[warn]                                                                          ^
[warn] 5 warnings found
[info] Done compiling.
[info] Compiling 10 Scala sources to /home/projects/oss/outwatch/outwatch/target/scala-2.12/test-classes ...
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/LifecycleHookSpec.scala:3:20: Unused import
[warn] import cats.effect.IO
[warn]                    ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:11:28: Unused import
[warn] import monix.execution.Ack.Continue
[warn]                            ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:2070:11: local val child in value $anonfun is never used
[warn]       val child = element.children(0)
[warn]           ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:2822:11: local val element in value $anonfun is never used
[warn]       val element = document.getElementById("strings")
[warn]           ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:2872:11: local val element in value $anonfun is never used
[warn]       val element = document.getElementById("strings")
[warn]           ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutwatchSpec.scala:37:11: private method getItem in <$anon: scala.scalajs.js.Object> is never used
[warn]       def getItem(key: String): String = map.getOrElse(key, null)
[warn]           ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutwatchSpec.scala:40:11: private method setItem in <$anon: scala.scalajs.js.Object> is never used
[warn]       def setItem(key: String, value: String): Unit = {
[warn]           ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutwatchSpec.scala:45:11: private method removeItem in <$anon: scala.scalajs.js.Object> is never used
[warn]       def removeItem(key: String): Unit = {
[warn]           ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutwatchSpec.scala:50:11: private method clear in <$anon: scala.scalajs.js.Object> is never used
[warn]       def clear(): Unit = map.clear()
[warn]           ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:2914:58: method toIO in trait Extensions is deprecated (since 3.0.0-RC3): Switch to task.to[IO]
[warn]       _ <- monix.eval.Task.unit.delayResult(0.1 seconds).toIO
[warn]                                                          ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:2918:58: method toIO in trait Extensions is deprecated (since 3.0.0-RC3): Switch to task.to[IO]
[warn]       _ <- monix.eval.Task.unit.delayResult(0.1 seconds).toIO
[warn]                                                          ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:2951:58: method toIO in trait Extensions is deprecated (since 3.0.0-RC3): Switch to task.to[IO]
[warn]       _ <- monix.eval.Task.unit.delayResult(0.1 seconds).toIO
[warn]                                                          ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:2955:58: method toIO in trait Extensions is deprecated (since 3.0.0-RC3): Switch to task.to[IO]
[warn]       _ <- monix.eval.Task.unit.delayResult(0.1 seconds).toIO
[warn]                                                          ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/ScenarioTestSpec.scala:25:32: method merge in trait ObservableDeprecatedBuilders is deprecated (since 3.0.0): Switch to Observable(list).merge
[warn]             count = Observable.merge(plusOne, minusOne).scan(0)(_ + _).startWith(Seq(0))
[warn]                                ^
[warn] 14 warnings found
[info] Done compiling.

Below are a bunch of warnings that have been fixed
sbt:OutWatch> test
[info] Updating ...
[info] Done updating.
[warn] There may be incompatibilities among your library dependencies; run 'evicted' to see detailed eviction warnings.
[info] Compiling 30 Scala sources to /home/projects/oss/outwatch/outwatch/target/scala-2.12/classes ...
[warn] /home/projects/oss/outwatch/outwatch/src/main/scala/outwatch/dom/OutwatchAttributes.scala:28:33: discarded non-Unit value
[warn]     obs => f((o,p) => obs.onNext((o.elm.toOption, p.elm.toOption)))
[warn]                                 ^
[warn] /home/projects/oss/outwatch/outwatch/src/main/scala/outwatch/dom/helpers/EmitterBuilder.scala:37:152: discarded non-Unit value
[warn]   @inline def apply[E <: Event](eventType: String): CustomEmitterBuilder[E, VDomModifier] = ofModifier[E](obs => Emitter(eventType, event => obs.onNext(event.asInstanceOf[E])))
[warn]                                                                                                                                                        ^
[warn] /home/projects/oss/outwatch/outwatch/src/main/scala/outwatch/dom/helpers/EmitterBuilder.scala:6:33: Unused import
[warn] import monix.reactive.observers.Subscriber
[warn]                                 ^
[warn] /home/projects/oss/outwatch/outwatch/src/main/scala/outwatch/dom/helpers/Snabbdom.scala:3:28: Unused import
[warn] import monix.execution.Ack.Continue
[warn]                            ^
[warn] /home/projects/oss/outwatch/outwatch/src/main/scala/outwatch/util/Store.scala:49:74: method fromIO in trait ObservableDeprecatedBuilders is deprecated (since 3.0.0): Switch to Observable.from
[warn]       (newState, effect.fold[Observable[A]](Observable.empty)(Observable.fromIO))
[warn]                                                                          ^
[warn] 5 warnings found
[info] Done compiling.
[info] Compiling 10 Scala sources to /home/projects/oss/outwatch/outwatch/target/scala-2.12/test-classes ...
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/LifecycleHookSpec.scala:3:20: Unused import
[warn] import cats.effect.IO
[warn]                    ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:11:28: Unused import
[warn] import monix.execution.Ack.Continue
[warn]                            ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:2070:11: local val child in value $anonfun is never used
[warn]       val child = element.children(0)
[warn]           ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:2822:11: local val element in value $anonfun is never used
[warn]       val element = document.getElementById("strings")
[warn]           ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:2872:11: local val element in value $anonfun is never used
[warn]       val element = document.getElementById("strings")
[warn]           ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutwatchSpec.scala:37:11: private method getItem in <$anon: scala.scalajs.js.Object> is never used
[warn]       def getItem(key: String): String = map.getOrElse(key, null)
[warn]           ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutwatchSpec.scala:40:11: private method setItem in <$anon: scala.scalajs.js.Object> is never used
[warn]       def setItem(key: String, value: String): Unit = {
[warn]           ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutwatchSpec.scala:45:11: private method removeItem in <$anon: scala.scalajs.js.Object> is never used
[warn]       def removeItem(key: String): Unit = {
[warn]           ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutwatchSpec.scala:50:11: private method clear in <$anon: scala.scalajs.js.Object> is never used
[warn]       def clear(): Unit = map.clear()
[warn]           ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:2914:58: method toIO in trait Extensions is deprecated (since 3.0.0-RC3): Switch to task.to[IO]
[warn]       _ <- monix.eval.Task.unit.delayResult(0.1 seconds).toIO
[warn]                                                          ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:2918:58: method toIO in trait Extensions is deprecated (since 3.0.0-RC3): Switch to task.to[IO]
[warn]       _ <- monix.eval.Task.unit.delayResult(0.1 seconds).toIO
[warn]                                                          ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:2951:58: method toIO in trait Extensions is deprecated (since 3.0.0-RC3): Switch to task.to[IO]
[warn]       _ <- monix.eval.Task.unit.delayResult(0.1 seconds).toIO
[warn]                                                          ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/OutWatchDomSpec.scala:2955:58: method toIO in trait Extensions is deprecated (since 3.0.0-RC3): Switch to task.to[IO]
[warn]       _ <- monix.eval.Task.unit.delayResult(0.1 seconds).toIO
[warn]                                                          ^
[warn] /home/projects/oss/outwatch/outwatch/src/test/scala/outwatch/ScenarioTestSpec.scala:25:32: method merge in trait ObservableDeprecatedBuilders is deprecated (since 3.0.0): Switch to Observable(list).merge
[warn]             count = Observable.merge(plusOne, minusOne).scan(0)(_ + _).startWith(Seq(0))
[warn]                                ^
[warn] 14 warnings found
[info] Done compiling.
@@ -86,15 +86,14 @@ object Store {
}).get
}

val sub = subject.subscribe()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forgot about this - I was trying to figure out why the initial action passed is not actually dispatched on the store that's created. I think it has something to do with replay(1).refCount. These methods are marked as side effecting, and at the time of their calls there will be no subscribers. I was not able to get this to work tho

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah, the store is sadly not perfect yet (also see #289). Ideas and PRs are definitely welcome.

Can you open an issue with your example?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this was just my first step trying to peer into why this is happening. My issue I think is the same as #289, I brought it up in the gitter:

https://gitter.im/OutWatch/Lobby?at=5d39c13b9114f065e2c89953

Copy link
Member

@cornerman cornerman left a comment

Choose a reason for hiding this comment

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

Nice, thank you very much!

@cornerman
Copy link
Member

onNext creates a future, but I don't see any way that this future is waited on, as it's not returned or awaited.

You are raising a very valid point here. The Future contains the Ack of the observer and builds up the backpressure protocol from monix. Currently, we really ignore it and just pass it down the Observer. So, in a sense the EmitterBuilder do not behave correctly. I have opened an issue for this (see #323).

just unclear to me how this sideeffect is ok in this context.

Internally, we do a lot of sideeffects and have optimized some IO away for performance reasons. This sideeffect is okay, because the emitter function is only ever called, if you have run your OutWatch.render. The sad part is that Emitter is also part of the public API, though normally you do not work with type Emitter but just use VDomModifier and let OutWatch interpret the modifiers.

@cornerman cornerman merged commit edfbb93 into outwatch:master Jul 25, 2019
cornerman pushed a commit to cornerman/outwatch that referenced this pull request Sep 24, 2021
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